When I filed the Jira issue I was planning to give an error, but I discovered that several dialects implement this cast, so my PR actually implements the behavior from Postgres.
I will amend the Jira case when we decide what the implementation should do. I can change the corresponding PR to give an error instead, but that is not a solution for the Pinot implementation either. It would be great if [CALCITE-3550] provides a solution for them. Mihai ________________________________ From: Julian Hyde <[email protected]> Sent: Thursday, February 22, 2024 4:16 PM To: [email protected] <[email protected]> Subject: Re: Remove a specific type coersion rule When I read CALCITE-6210 it wasn’t clear that you wanted Calcite to give a validation error when someone tries to cast a VARCHAR to a VARBINARY. I agree that that is the desirable behavior. (And I believe it is consistent with the SQL standard.) Can you amend the Jira case so that it clearly the goal? Reading the code, I see that https://issues.apache.org/jira/browse/CALCITE-3550 was trying to provide a way for people to change the coercion rules. Does that solution meet your needs? Julian > On Feb 22, 2024, at 6:59 AM, Gonzalo Ortiz Jaureguizar <[email protected]> > wrote: > > Hello there, > > In the context of https://issues.apache.org/jira/browse/CALCITE-6210, the > Apache Pinot team is thinking about forbidding casting from VARCHAR to > VARBINARY. > > I've been trying to implement that, but I'm not sure if it is possible or > not. Following the Javadoc of SqlTypeCoercionRule (which, btw, seems a bit > outdated) I've tried to create my own coercion rule as: > > ``` > private static SqlTypeCoercionRule createPinotCoercionRule() { > // Initialize a Builder instance with the default mappings. > Map<SqlTypeName, ImmutableSet<SqlTypeName>> pinotTypeMapping = new > HashMap<>( > SqlTypeCoercionRule.instance().getTypeMapping() > ); > pinotTypeMapping.put(SqlTypeName.BINARY, > ImmutableSet.of(SqlTypeName.VARBINARY)); > pinotTypeMapping.put(SqlTypeName.VARBINARY, > ImmutableSet.of(SqlTypeName.BINARY)); > > // Initialize a SqlTypeCoercionRules with the new builder mappings. > return SqlTypeCoercionRule.instance(pinotTypeMapping); > } > ``` > > Then I've tried to execute a query like: `select 1 from Table where > varBinaryField = 'some text'` and even when that SqlTypeCoercionRule is > used, the Validator turns that into `select 1 from Table where > varBinaryField = cast('some text'` as VARBINARY)`, which should be illegal. > That expression is then simplified when transformed into a RelRoot and then > the error described in https://issues.apache.org/jira/browse/CALCITE-6210 > is thrown. Same cast is added with other queries like `select 1 from Table > where OCTET_LENGTH('80c062bc98021f94f1404e9bda0f6b0202') > 0`. > > It seems that the reason why this CAST is being added is because > AbstractTypeCoersion.commonTypeForBinaryComparison and > AbstractTypeCoersion.implicitCast assume some coercions are always valid. > > The question then is: Is this working as expected? Should we assume that > rules can be added to SqlTypeCoercionRule but they cannot be not removed? > What are the alternatives we have if we want to be more restrictive than > the castings explained in > https://docs.google.com/spreadsheets/d/1GhleX5h5W8-kJKh7NMJ4vtoE78pwfaZRJl88ULX_MgU > ? > > It would be fine to me if I could add extra enforcement at validation time. > Specifically, if that enforcement could be added after Validator modified > the AST so I can be sure it will catch any possible CAST. I can do that in > a RelOptRule, but it would be better to enforce the restriction in SqlNode > in order to be able to include in the error message the position in the > original expression. > > Bests > > Gonzalo
