Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18999 )
Change subject: IMPALA-10753: Incorrect length when multiple CHAR(N) values are inserted ...................................................................... Patch Set 9: (7 comments) http://gerrit.cloudera.org:8080/#/c/18999/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18999/8//COMMIT_MSG@8 PS8, Line 8: : If, in a VALUES clause, for the same column all of the values are CHAR : types but not all are of the same length, the common type chosen is : CHAR(max(lengths)). This means that shorter values are padded with : spaces. If the destination column is not CHAR but VARCHAR or STRING, : this produces different results than if the values in the column are : inserted individually, in separate statements. This behaviour is : suboptimal because information is lost. > An example could make this clearer. Done http://gerrit.cloudera.org:8080/#/c/18999/8//COMMIT_MSG@17 PS8, Line 17: > I would prefer to add CHAR to the name somehow as it is only applied for th Done http://gerrit.cloudera.org:8080/#/c/18999/8//COMMIT_MSG@26 PS8, Line 26: > Can you add a note about already different behavior in CHAR(N) padding comp I mentioned Hive. If it is important I can find out what Postgres does. http://gerrit.cloudera.org:8080/#/c/18999/8/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/18999/8/common/thrift/Query.thrift@646 PS8, Line 646: > nit: false Done http://gerrit.cloudera.org:8080/#/c/18999/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java File fe/src/main/java/org/apache/impala/analysis/StatementBase.java: http://gerrit.cloudera.org:8080/#/c/18999/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@200 PS8, Line 200: > This is no longer true, right? Right, removed this from the comment. http://gerrit.cloudera.org:8080/#/c/18999/8/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java File fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java: http://gerrit.cloudera.org:8080/#/c/18999/8/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java@146 PS8, Line 146: : : : : : : : : : : : : : : : : > Calling this before analyze looks a bit convoluted. Done http://gerrit.cloudera.org:8080/#/c/18999/8/testdata/workloads/functional-query/queries/QueryTest/chars-values-stmt-non-lossy-common-type.test File testdata/workloads/functional-query/queries/QueryTest/chars-values-stmt-non-lossy-common-type.test: http://gerrit.cloudera.org:8080/#/c/18999/8/testdata/workloads/functional-query/queries/QueryTest/chars-values-stmt-non-lossy-common-type.test@32 PS8, Line 32: > Can you also add longer string that will be truncated? If the value is longer than the destination field, it results in an error. I added a test case for that. -- To view, visit http://gerrit.cloudera.org:8080/18999 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e9e189cb3c2be0e741ca3d15a7f97ec3a1b1a86 Gerrit-Change-Number: 18999 Gerrit-PatchSet: 9 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Peter Rozsa <pro...@cloudera.com> Gerrit-Comment-Date: Tue, 04 Jul 2023 11:13:41 +0000 Gerrit-HasComments: Yes
