You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/11/07 07:01:06 UTC

[GitHub] [inlong] thesumery opened a new pull request, #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

thesumery opened a new pull request, #6424:
URL: https://github.com/apache/inlong/pull/6424

   [INLONG-6409][Sort] Bugfix:unspported Time and Timestamp iceberg auto create table in spark session query
   
   ### Prepare a Pull Request
   *(Change the title refer to the following example)*
   
   - [INLONG-6409][Sort] Bugfix:unspported Time and Timestamp iceberg auto create table in spark session query
   - Fixes #6409 
   
   ### Motivation
   
   *Spported Time and Timestamp iceberg auto create table in spark session query*
   
   ### Modifications
   
   *convert time to string and timestamp to timestamp with local zone.*
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] dockerzhang merged pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
dockerzhang merged PR #6424:
URL: https://github.com/apache/inlong/pull/6424


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] yunqingmoswu commented on a diff in pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
yunqingmoswu commented on code in PR #6424:
URL: https://github.com/apache/inlong/pull/6424#discussion_r1015143803


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/format/DebeziumJsonDynamicSchemaFormat.java:
##########
@@ -289,16 +281,6 @@ public List<RowData> extractRowData(JsonNode data, RowType rowType) {
         return extractRowData(payload, rowType);
     }
 
-    /**
-     * Get the identifier of this dynamic schema format
-     *
-     * @return The identifier of this dynamic schema format
-     */
-    @Override
-    public String identifier() {

Review Comment:
   Maybe it is better to keep the  'identifier()'?



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/format/DebeziumJsonDynamicSchemaFormat.java:
##########
@@ -44,7 +44,6 @@
  */
 public class DebeziumJsonDynamicSchemaFormat extends JsonDynamicSchemaFormat {
 
-    private static final String IDENTIFIER = "debezium-json";

Review Comment:
   Maybe it is better to keep the 'IDENTIFIER'?



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/Constants.java:
##########
@@ -163,4 +163,11 @@ public final class Constants {
                     .booleanType()
                     .defaultValue(true)
                     .withDescription("Whether ignore the single table erros when multiple sink writing scenario.");
+
+    public static final ConfigOption<Boolean> SINK_MULTIPLE_TYPE_MAP_SPARK_ENGINE_ENABLE =
+            ConfigOptions.key("sink.multiple.typemap.spark.engine.enable")

Review Comment:
   Maybe it is better to call 'sink.multiple.typemap-compatible-with-spark'?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] EMsnap commented on a diff in pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
EMsnap commented on code in PR #6424:
URL: https://github.com/apache/inlong/pull/6424#discussion_r1016187735


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/Constants.java:
##########
@@ -163,4 +163,11 @@ public final class Constants {
                     .booleanType()
                     .defaultValue(true)
                     .withDescription("Whether ignore the single table erros when multiple sink writing scenario.");
+
+    public static final ConfigOption<Boolean> SINK_MULTIPLE_TYPE_MAP_COMPATIBLE_WITH_SPARK =
+            ConfigOptions.key("sink.multiple.typemap-compatible-with-spark")
+                    .booleanType()
+                    .defaultValue(false)
+                    .withDescription("Because spark do not support iceberg data type: `timestamp without time zone` and"
+                            + "`time`, so type conversions must be mapped to types supported by spark.");

Review Comment:
   maped to string ? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] EMsnap commented on a diff in pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
EMsnap commented on code in PR #6424:
URL: https://github.com/apache/inlong/pull/6424#discussion_r1016189560


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/Constants.java:
##########
@@ -163,4 +163,11 @@ public final class Constants {
                     .booleanType()
                     .defaultValue(true)
                     .withDescription("Whether ignore the single table erros when multiple sink writing scenario.");
+
+    public static final ConfigOption<Boolean> SINK_MULTIPLE_TYPE_MAP_COMPATIBLE_WITH_SPARK =
+            ConfigOptions.key("sink.multiple.typemap-compatible-with-spark")
+                    .booleanType()
+                    .defaultValue(false)
+                    .withDescription("Because spark do not support iceberg data type: `timestamp without time zone` and"
+                            + "`time`, so type conversions must be mapped to types supported by spark.");

Review Comment:
   I guess it's nessasary to mention the conversion explicitly like datetime to varchar



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] EMsnap commented on a diff in pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
EMsnap commented on code in PR #6424:
URL: https://github.com/apache/inlong/pull/6424#discussion_r1016189560


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/Constants.java:
##########
@@ -163,4 +163,11 @@ public final class Constants {
                     .booleanType()
                     .defaultValue(true)
                     .withDescription("Whether ignore the single table erros when multiple sink writing scenario.");
+
+    public static final ConfigOption<Boolean> SINK_MULTIPLE_TYPE_MAP_COMPATIBLE_WITH_SPARK =
+            ConfigOptions.key("sink.multiple.typemap-compatible-with-spark")
+                    .booleanType()
+                    .defaultValue(false)
+                    .withDescription("Because spark do not support iceberg data type: `timestamp without time zone` and"
+                            + "`time`, so type conversions must be mapped to types supported by spark.");

Review Comment:
   I guess it's nessasary to mention the conversion explicitly like time to varchar



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] thesumery commented on pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
thesumery commented on PR #6424:
URL: https://github.com/apache/inlong/pull/6424#issuecomment-1306741523

   Wait to merge


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] thesumery commented on a diff in pull request #6424: [INLONG-6409][Sort] Unspported Time and Timestamp iceberg auto create table in spark session query

Posted by GitBox <gi...@apache.org>.
thesumery commented on code in PR #6424:
URL: https://github.com/apache/inlong/pull/6424#discussion_r1016115407


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/format/DebeziumJsonDynamicSchemaFormat.java:
##########
@@ -44,7 +44,6 @@
  */
 public class DebeziumJsonDynamicSchemaFormat extends JsonDynamicSchemaFormat {
 
-    private static final String IDENTIFIER = "debezium-json";

Review Comment:
   'IDENTIFIER' is used for distinguish different DynamicSchemaFormat,but 'map<String, ?>'  keys can achieve the same goal. Here if want to keep 'IDENTIFIER', must modify getIdentifier to static function because it is private.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org