Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20425 )
Change subject: IMPALA-10086: Implicit cast comparing char and varchar ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/20425/2/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/20425/2/fe/src/main/java/org/apache/impala/analysis/Expr.java@1576 PS2, Line 1576: if (type.matchesType(targetType)) return uncheckedCastTo(type, compatibility); I've updated this to a version that I think makes better semantic sense (matching the comment) and passes the new and existing cast tests (still need to run the full test suite). This preserves the prior behavior around targetType.equals(type) (as long as equals is symmetric). matchesType adds the additional semantic that if type is char/varchar/decimal and targetType is a wildcard version of same, then they match and we cast to the compatible (non-wildcard) version. Casting to targetType tends to not work or crash in the backend, so I suspect representation of wildcard varchar is incomplete (if you create a table with a 'varchar' value, it represents it as STRING). Casting to the compatible type (which must have a length) makes sense to me. I still plan to track down what change broke this behavior. http://gerrit.cloudera.org:8080/#/c/20425/1/fe/src/main/java/org/apache/impala/catalog/DefaultCompatibility.java File fe/src/main/java/org/apache/impala/catalog/DefaultCompatibility.java: http://gerrit.cloudera.org:8080/#/c/20425/1/fe/src/main/java/org/apache/impala/catalog/DefaultCompatibility.java@151 PS1, Line 151: > Looks like I need to add a test case covering that too. Done http://gerrit.cloudera.org:8080/#/c/20425/1/tests/query_test/test_cast_with_format.py File tests/query_test/test_cast_with_format.py: http://gerrit.cloudera.org:8080/#/c/20425/1/tests/query_test/test_cast_with_format.py@2189 PS1, Line 2189: self > Ack Done -- To view, visit http://gerrit.cloudera.org:8080/20425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib89d0a391bc8f2152ecd9151c8872a01ba19c436 Gerrit-Change-Number: 20425 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Wed, 06 Sep 2023 00:37:55 +0000 Gerrit-HasComments: Yes