Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/20184 )
Change subject: IMPALA-10173: (Addendum) Fix substitution for unsafe expressions, column-level compatibility check ...................................................................... Patch Set 4: Code-Review+1 (4 comments) Thanks, only some nits. http://gerrit.cloudera.org:8080/#/c/20184/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20184/4//COMMIT_MSG@18 PS4, Line 18: Set Could start it with "Also, ..." to indicate that this is an additional issue that this change resolves. http://gerrit.cloudera.org:8080/#/c/20184/4/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test File testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test: http://gerrit.cloudera.org:8080/#/c/20184/4/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test@214 PS4, Line 214: # Mixing unsafe and regular compatibility on column level, unsafe union between 1 (TINYINT) and "1" (STRING) Nit: it would be better to keep the line width of the comment to 90. http://gerrit.cloudera.org:8080/#/c/20184/4/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test@215 PS4, Line 215: " Nit: full stop (.) at the end. http://gerrit.cloudera.org:8080/#/c/20184/4/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test@219 PS4, Line 219: casts Nit: full stop (.) at the end. -- To view, visit http://gerrit.cloudera.org:8080/20184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39d13f177482f74ec39570118adab609444c6929 Gerrit-Change-Number: 20184 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Rozsa <pro...@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: Thu, 27 Jul 2023 10:39:35 +0000 Gerrit-HasComments: Yes
