Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22289 )
Change subject: IMPALA-12927: Support specifying format for reading JSON BINARY columns ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/22289/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/22289/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@567 PS6, Line 567: TJsonBinaryFormat specificFormat = partition.getInputFormatDescriptor() Will this return the table level serde property in a partitioned table where it was set at a table level? It would be better if there was a test with a partitioned table. I think that it is actually ok to forbid the case when different partitions have different JSON_BINARY_FORMAT to simplify testing. It could be added later if someone actuallyn needs it. http://gerrit.cloudera.org:8080/#/c/22289/6/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: http://gerrit.cloudera.org:8080/#/c/22289/6/testdata/datasets/functional/functional_schema_template.sql@4499 PS6, Line 4499: alter table {db_name}{db_suffix}.{table_name} set serdeproperties ('json.binary.format'='base64'); It seems cleaner to me to do the ALTER before the INSERT - or that would lead to a different result? http://gerrit.cloudera.org:8080/#/c/22289/6/testdata/datasets/functional/functional_schema_template.sql@4544 PS6, Line 4544: alter table {db_name}{db_suffix}.{table_name} partition(year) set serdeproperties ('json.binary.format'='base64'); Same as line 4499? http://gerrit.cloudera.org:8080/#/c/22289/6/testdata/workloads/functional-query/queries/QueryTest/json-binary-format.test File testdata/workloads/functional-query/queries/QueryTest/json-binary-format.test: http://gerrit.cloudera.org:8080/#/c/22289/6/testdata/workloads/functional-query/queries/QueryTest/json-binary-format.test@72 PS6, Line 72: # Unsetting the property and setting the query option to 'base64' will have the same effect. Nit: can you break the lines to fit into column width 90? This is not needed ---- CATCH strings, but should be done query text/comments -- To view, visit http://gerrit.cloudera.org:8080/22289 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf61fa3afc0f33caa63fbc05393e975733165e82 Gerrit-Change-Number: 22289 Gerrit-PatchSet: 6 Gerrit-Owner: Zihao Ye <eyiz...@163.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zihao Ye <eyiz...@163.com> Gerrit-Comment-Date: Tue, 18 Mar 2025 07:47:26 +0000 Gerrit-HasComments: Yes
