[
https://issues.apache.org/jira/browse/CALCITE-7277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dmitry Sysolyatin updated CALCITE-7277:
---------------------------------------
Description:
Currently, almost all of the Calcite codebase and its tests use an overridden
version of SqlValidator.Config.DEFAULT. Example from PlannerImpl:
{code}
sqlValidatorConfig
.withDefaultNullCollation(connectionConfig.defaultNullCollation())
.withLenientOperatorLookup(connectionConfig.lenientOperatorLookup())
.withConformance(connectionConfig.conformance())
.withIdentifierExpansion(true));
{code}
Because of this, the actual {{SqlValidator.Config.DEFAULT}} (which uses
withIdentifierExpansion(false)) is mostly untested. When people integrate
Calcite components into their products using the default configuration, they
often encounter the same issues, we still have several bugs related to
withIdentifierExpansion(false). As a result, users simply change their code to
use {{SqlValidator.Config.DEFAULT.withIdentifierExpansion(true)}} or something
similar, and the underlying issues remain unfixed.
There are two ways to fix this (probably more):
1. Change the default setting. Use the same overrides that we already apply in
tests and in most Calcite components (like PlannerImpl). I already sent an
email about setting withIdentifierExpansion(true) as the default [1], but there
weren’t many replies. If user override DEFAULT, then it is on their own risk to
do it
2. Improve test coverage for SqlValidator.Config.DEFAULT. Make sure the default
configuration is well tested. We could do this by running tests like
SqlToRelConverter and others with multiple configurations instead of just one.
However, this is not as easy as it sounds, because it may reveal many existing
issues.
[1] https://lists.apache.org/thread/h5f83zc7y7dpr28tm7wthy2jgbhzbhjx
was:
Currently, almost all of the Calcite codebase and its tests use an overridden
version of SqlValidator.Config.DEFAULT. Example from PlannerImpl:
{code}
sqlValidatorConfig
.withDefaultNullCollation(connectionConfig.defaultNullCollation())
.withLenientOperatorLookup(connectionConfig.lenientOperatorLookup())
.withConformance(connectionConfig.conformance())
.withIdentifierExpansion(true));
{code}
Because of this, the actual {{SqlValidator.Config.DEFAULT}} (which uses
withIdentifierExpansion(false)) is mostly untested. When people integrate
Calcite components into their products using the default configuration, they
often encounter the same issues, we still have several bugs related to
withIdentifierExpansion(false).
As a result, many users simply change their code to use
{{SqlValidator.Config.DEFAULT.withIdentifierExpansion(true)}} or something
similar, and the underlying issues remain unfixed.
There are two ways to fix this (probably more):
1. Change the default setting. Use the same overrides that we already apply in
tests and in most Calcite components (like PlannerImpl). I already sent an
email about setting withIdentifierExpansion(true) as the default [1], but there
weren’t many replies. If user override DEFAULT, then it is on their own risk to
do it
2. Improve test coverage for SqlValidator.Config.DEFAULT. Make sure the default
configuration is well tested. We could do this by running tests like
SqlToRelConverter and others with multiple configurations instead of just one.
However, this is not as easy as it sounds, because it may reveal many existing
issues.
[1] https://lists.apache.org/thread/h5f83zc7y7dpr28tm7wthy2jgbhzbhjx
> Improve test coverage for SqlValidator.Config.DEFAULT
> -----------------------------------------------------
>
> Key: CALCITE-7277
> URL: https://issues.apache.org/jira/browse/CALCITE-7277
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Affects Versions: 1.41.0
> Reporter: Dmitry Sysolyatin
> Priority: Major
>
> Currently, almost all of the Calcite codebase and its tests use an overridden
> version of SqlValidator.Config.DEFAULT. Example from PlannerImpl:
> {code}
> sqlValidatorConfig
> .withDefaultNullCollation(connectionConfig.defaultNullCollation())
>
> .withLenientOperatorLookup(connectionConfig.lenientOperatorLookup())
> .withConformance(connectionConfig.conformance())
> .withIdentifierExpansion(true));
> {code}
> Because of this, the actual {{SqlValidator.Config.DEFAULT}} (which uses
> withIdentifierExpansion(false)) is mostly untested. When people integrate
> Calcite components into their products using the default configuration, they
> often encounter the same issues, we still have several bugs related to
> withIdentifierExpansion(false). As a result, users simply change their code
> to use {{SqlValidator.Config.DEFAULT.withIdentifierExpansion(true)}} or
> something similar, and the underlying issues remain unfixed.
> There are two ways to fix this (probably more):
> 1. Change the default setting. Use the same overrides that we already apply
> in tests and in most Calcite components (like PlannerImpl). I already sent an
> email about setting withIdentifierExpansion(true) as the default [1], but
> there weren’t many replies. If user override DEFAULT, then it is on their own
> risk to do it
> 2. Improve test coverage for SqlValidator.Config.DEFAULT. Make sure the
> default configuration is well tested. We could do this by running tests like
> SqlToRelConverter and others with multiple configurations instead of just
> one. However, this is not as easy as it sounds, because it may reveal many
> existing issues.
> [1] https://lists.apache.org/thread/h5f83zc7y7dpr28tm7wthy2jgbhzbhjx
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
