[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters ...................................................................... Patch Set 12: (16 comments) > Uploaded patch set 12. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@9 PS11, Line 9: An > An error - we're introducing it now. Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@14 PS11, Line 14: matched one of th > matched one of the ... Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@15 PS11, Line 15: '\u00FF' > '\u00FF' is not a valid character literal in C++. "\u00FF" was included in Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@18 PS11, Line 18: A set > See the comment above, we should explain that it was a mistake from the beg Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: > Parquet and Iceberg are not exclusive things, we also use Parquet in our Ic Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: > It would be good to include in which file the new tests are. Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@51 PS11, Line 51: '\x1A > Why adding space character here? Hive doesn't escape it in partition paths. Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@57 PS11, Line 57: "upp > It would be clearer like this: "uppercase" and "hex" only affect the insert Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@62 PS11, Line 62: // the character is not alphanumeric and it is not one of the characters specifically > This doesn't match the code. Try this: Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test File testdata/workloads/functional-query/queries/QueryTest/insert.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@450 PS11, Line 450: 'R > We just need double single quotes in the RESULTS section to escape a single Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@452 PS11, Line 452: > I don't think need this. It's used for non-ascii characters. Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@470 PS11, Line 470: > This will fail if we escape space. Ack http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test File testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@327 PS11, Line 327: ---- TYPES > You don't need to drop the table, the tables are created in a unique databa Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@337 PS11, Line 337: > I don't think this is needed. Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@344 PS11, Line 344: > You should do the same insertions into the Iceberg table as into the other Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@354 PS11, Line 354: > See L327. Done -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 12 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zihao Ye <[email protected]> Gerrit-Comment-Date: Tue, 30 Apr 2024 05:14:02 +0000 Gerrit-HasComments: Yes
