[GitHub] [calcite-avatica] libenchao closed pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
libenchao closed pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields URL: https://github.com/apache/calcite-avatica/pull/183 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1003932604
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
Per the changes in this PR, you are right that the semantic for `inQuery`
does not change too much, I'm fine to leave it is for now.
The concern I raised is that `FrameTypeEnum#SELECT` is used vaguely to my
understanding, e.g., we both use `SELECT` frame in `SqlInsert` and
`SqlSelectOperator`, but they are different in my opinion: `SqlInsert` use
`SELECT` frame to indicate that we'll go to a `SELECT QUERY`, and
`SqlSelectOperator` use `SELECT` frame to indicate that we are in a `SELECT
QUERY`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] F21 opened a new pull request, #187: [CALCITE-5344] Remove Travis CI configuration
F21 opened a new pull request, #187: URL: https://github.com/apache/calcite-avatica/pull/187 The ASF is discontinuing Travis CI for testing, and it will no longer be available after 31 December 2022. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] F21 opened a new pull request, #2950: [CALCITE-5344] Remove Travis CI configuration
F21 opened a new pull request, #2950: URL: https://github.com/apache/calcite/pull/2950 The ASF is discontinuing Travis CI for testing, and it will no longer be available after 31 December 2022. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1003928315
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -3749,26 +3774,26 @@ void checkPeriodPredicate(Checker checker) {
+ "select * from e except "
+ "select * from f union "
+ "select * from g";
-final String expected = "SELECT *\n"
+final String expected = "SELECT *\n"
Review Comment:
Since we are changing `SET_QUERY` to non-expression, could you also add a
test case to make sure the precedence for set operators could work well? such
as `(select * from a union select * from b) intersect select * from c` should
be unparsed as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] libenchao commented on pull request #2948: [CALCITE-5340] Tests should fail when actual and expected XML reference files are not identical
libenchao commented on PR #2948: URL: https://github.com/apache/calcite/pull/2948#issuecomment-1289869013 @asolimando The PR looks great. The only question I got is why `TopDownOptTest` and `RuleMatchVisualizerTest` are not touched in this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2686: [CALCITE-4982] NonNull field shouldn't be pushed down into leaf of outer-join in 'ProjectJoinTransposeRule'
libenchao commented on code in PR #2686:
URL: https://github.com/apache/calcite/pull/2686#discussion_r1003909401
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java:
##
@@ -157,7 +159,24 @@ public ProjectJoinTransposeRule(
@Value.Immutable(singleton = false)
public interface Config extends RelRule.Config {
Config DEFAULT = ImmutableProjectJoinTransposeRule.Config.builder()
-.withPreserveExprCondition(expr -> !(expr instanceof RexOver))
+.withPreserveExprCondition(expr -> {
+ // Non push down over's expression by default
Review Comment:
kindly ping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] wnob opened a new pull request, #186: Add proper support for timestamps with timezones
wnob opened a new pull request, #186: URL: https://github.com/apache/calcite-avatica/pull/186 According to the [JDBC 4.2 spec][1], the `TIME_WITH_TIMEZONE` and `TIMESTAMP_WITH_TIMEZONE` types are meant to be accessed as `java.time.OffsetTime` and `java.time.OffsetDateTime` objects respectively, via the `setObject` and `getObject` methods on result sets. This PR is meant to implement this proper behavior, as well as adding the convenience to use `getString` (returning ISO 8601-encoded strings) as well as `getTime` and `getTimestamp` respectively (returning the same point in time as a timezone-less `java.util.Date` derivative, as though it were a simple `TIME` or `TIMESTAMP` JDBC type). The PR will still require a bit of workshopping, but I wanted to get your eyes on it now in case you have any comments on the approach so far. In particular, I want to make sure that calling `setString` on one of the timezone-attached types does not require parsing the string, because I believe ISO 8601 is the most appropriate serialization format for these types. Does this mean, in `TypedValue.java`, that I must pick `String` as the "Local" representation for these types? It would seem to be counter to the stated intention of it being "used by Calcite for efficient computation". [1]: https://download.oracle.com/otn-pub/jcp/jdbc-4_2-mrel2-spec/jdbc4.2-fr-spec.pdf -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] HanumathRao commented on pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
HanumathRao commented on PR #2935: URL: https://github.com/apache/calcite/pull/2935#issuecomment-1289138925 Thank you @zabetak for the review and improvements. Please go ahead and merge the PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando merged pull request #2947: [MINOR] Add default catalog link to test fixtures
asolimando merged PR #2947: URL: https://github.com/apache/calcite/pull/2947 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] wojustme commented on pull request #2686: [CALCITE-4982] NonNull field shouldn't be pushed down into leaf of outer-join in 'ProjectJoinTransposeRule'
wojustme commented on PR #2686: URL: https://github.com/apache/calcite/pull/2686#issuecomment-1288937219 In my opinion, this case only works on in CALCITE's optimizer or validator, which some expresion only change nullable attribute. we couldn't write an sql `cast(c1 as not integer)`, and we only write an sql `cast(c1 as integer)`. This pr will fix the issue with a little modification. Maybe we get a good solution from the discussion of [CALCITE-5315](https://issues.apache.org/jira/browse/CALCITE-5315). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
zabetak commented on PR #2935: URL: https://github.com/apache/calcite/pull/2935#issuecomment-1288891782 I pushed a few small changes to the PR. @HanumathRao let me know if you are OK with those so that I can rebase and merge the PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on a diff in pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
zabetak commented on code in PR #2935: URL: https://github.com/apache/calcite/pull/2935#discussion_r1003201432 ## core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml: ## @@ -3058,6 +3058,50 @@ LogicalSort(sort0=[$7], dir0=[ASC], fetch=[0]) + + + + + + + + + + + + + + + + + + + + + +
[GitHub] [calcite] asolimando commented on pull request #2947: [MINOR] Add default catalog link to test fixtures
asolimando commented on PR #2947: URL: https://github.com/apache/calcite/pull/2947#issuecomment-1288630911 Thanks @JiajunBernoulli, I agree on your reasoning. Can I ask you to squash the commits? Once done, I will merge it in the next 24 hours. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1003027206
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
You are right, exclude "SET_QUERY" from EXPRESSION will be better, but many
test cases may fail if do that, I will improve it in next commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1002851565
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
Actually, I think I have not extended the meaning of `SqlWriter#inQuery`.
Current java doc: "Returns whether we are currently in a query context" is
still appropriate now. Before this PR, `inQuery` covers "in WITH", I think
"WITH" operator should be split into two operators: WITH_ITEM and WITH_BODY, so
I change "WITH" to "WITH_BODY", that's the only change on this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] libenchao commented on a diff in pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
libenchao commented on code in PR #183:
URL: https://github.com/apache/calcite-avatica/pull/183#discussion_r1002804111
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -222,15 +223,22 @@ public static T instantiatePlugin(Class
pluginClass,
final Class clazz = (Class) Class.forName(left);
final Field field;
field = clazz.getField(right);
-return pluginClass.cast(field.get(null));
+final Object fieldValue = field.get(null);
+if (fieldValue instanceof ThreadLocal) {
+ value = ((ThreadLocal) fieldValue).get();
+} else {
+ value = fieldValue;
+}
+return pluginClass.cast(value);
}
//noinspection unchecked
final Class clazz = (Class) Class.forName(className);
try {
// We assume that if there is an INSTANCE field it is static and
// has the right type.
final Field field = clazz.getField("INSTANCE");
-return pluginClass.cast(field.get(null));
+value = field.get(null);
+return pluginClass.cast(value);
Review Comment:
Agreed, thanks for the explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] F21 commented on pull request #184: CALCITE-5327 Make SSL key-store type configurable
F21 commented on PR #184: URL: https://github.com/apache/calcite-avatica/pull/184#issuecomment-1288233986 +1 to @joshelser's comments, otherwise everything else looks great! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] julianhyde commented on a diff in pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
julianhyde commented on code in PR #183:
URL: https://github.com/apache/calcite-avatica/pull/183#discussion_r1002773357
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -211,6 +211,7 @@ public static T instantiatePlugin(Class pluginClass,
String className) {
String right = null;
String left = null;
+Object value = null;
Review Comment:
In fa3244a99 I have improved the javadoc for the 'Class#FOO' case.
I don't think we should allow `INSTANCE` to be a `ThreadLocal` because in
the singleton pattern INSTANCE should have the same type as its enclosing class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] julianhyde commented on a diff in pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
julianhyde commented on code in PR #183:
URL: https://github.com/apache/calcite-avatica/pull/183#discussion_r1002770970
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -222,15 +223,22 @@ public static T instantiatePlugin(Class
pluginClass,
final Class clazz = (Class) Class.forName(left);
final Field field;
field = clazz.getField(right);
-return pluginClass.cast(field.get(null));
+final Object fieldValue = field.get(null);
+if (fieldValue instanceof ThreadLocal) {
+ value = ((ThreadLocal) fieldValue).get();
+} else {
+ value = fieldValue;
+}
+return pluginClass.cast(value);
}
//noinspection unchecked
final Class clazz = (Class) Class.forName(className);
try {
// We assume that if there is an INSTANCE field it is static and
// has the right type.
final Field field = clazz.getField("INSTANCE");
-return pluginClass.cast(field.get(null));
+value = field.get(null);
+return pluginClass.cast(value);
Review Comment:
Good question, though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] julianhyde commented on a diff in pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
julianhyde commented on code in PR #183:
URL: https://github.com/apache/calcite-avatica/pull/183#discussion_r1002770793
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -222,15 +223,22 @@ public static T instantiatePlugin(Class
pluginClass,
final Class clazz = (Class) Class.forName(left);
final Field field;
field = clazz.getField(right);
-return pluginClass.cast(field.get(null));
+final Object fieldValue = field.get(null);
+if (fieldValue instanceof ThreadLocal) {
+ value = ((ThreadLocal) fieldValue).get();
+} else {
+ value = fieldValue;
+}
+return pluginClass.cast(value);
}
//noinspection unchecked
final Class clazz = (Class) Class.forName(className);
try {
// We assume that if there is an INSTANCE field it is static and
// has the right type.
final Field field = clazz.getField("INSTANCE");
-return pluginClass.cast(field.get(null));
+value = field.get(null);
+return pluginClass.cast(value);
Review Comment:
No. If a class `Foo` has a field called `INSTANCE`, it should be of type
`Foo`. It would be confusing for a field called `INSTANCE` to be of any other
type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #2949: [CALCITE-4455] Calcite SQLParser: Provide support for INSERT OVERWRITE
julianhyde commented on code in PR #2949:
URL: https://github.com/apache/calcite/pull/2949#discussion_r1002766746
##
babel/src/main/codegen/config.fmpp:
##
@@ -31,12 +31,14 @@ data: {
"org.apache.calcite.sql.babel.SqlBabelCreateTable",
"org.apache.calcite.sql.babel.TableCollectionType",
"org.apache.calcite.sql.ddl.SqlDdlNodes",
+ "org.apache.calcite.sql.babel.SqlInsertOverwriteTable",
Review Comment:
alphabetical order
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] zabetak closed pull request #185: [CALCITE-3078] Move public lastDay method from Calcite to Avatica
zabetak closed pull request #185: [CALCITE-3078] Move public lastDay method from Calcite to Avatica URL: https://github.com/apache/calcite-avatica/pull/185 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] zabetak closed pull request #28: [WIP][CALCITE-1806] Add Apache Spark JDBC test to Avatica server
zabetak closed pull request #28: [WIP][CALCITE-1806] Add Apache Spark JDBC test to Avatica server URL: https://github.com/apache/calcite-avatica/pull/28 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] zabetak commented on pull request #28: [WIP][CALCITE-1806] Add Apache Spark JDBC test to Avatica server
zabetak commented on PR #28: URL: https://github.com/apache/calcite-avatica/pull/28#issuecomment-1288156910 This PR has been inactive for quite some time now and it seems there is no interest from the authors to push this forward thus I am closing it down. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] zabetak closed pull request #98: [CALCITE-3078] Duplicate code lastDay in calcite-avatica and calcite
zabetak closed pull request #98: [CALCITE-3078] Duplicate code lastDay in calcite-avatica and calcite URL: https://github.com/apache/calcite-avatica/pull/98 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] zabetak commented on pull request #98: [CALCITE-3078] Duplicate code lastDay in calcite-avatica and calcite
zabetak commented on PR #98: URL: https://github.com/apache/calcite-avatica/pull/98#issuecomment-1288156204 Closing this in favor of https://github.com/apache/calcite-avatica/pull/185 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on pull request #2947: [MINOR] Add default catalog link to test fixtures
JiajunBernoulli commented on PR #2947: URL: https://github.com/apache/calcite/pull/2947#issuecomment-1288039472 Thanks for your review.@asolimando I agree with your suggestion and I use a single description: > Creates a test context with a SQL query. There are two reasons to use it: - I think it is more appropriate to stress SQL query, because `expr()` function description is > Creates a test context with a SQL expression. - **fixture** is abstract concept, **test context** is a better understanding vocabulary. > Creates a fixture and sets its SQL statement. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1002626396
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
I'm not sure about this, as I said above, I'm confused about `inQuery` now,
could you explain your understanding of this method?
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
> IMHO, SET_QUERY should not be a EXPRESSION.
Then you can add `SET_QUERY` to `SqlKind.EXPRESSION`, and you do not need to
add this condition `!writer.inWithBody()` here anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2686: [CALCITE-4982] NonNull field shouldn't be pushed down into leaf of outer-join in 'ProjectJoinTransposeRule'
libenchao commented on code in PR #2686:
URL: https://github.com/apache/calcite/pull/2686#discussion_r1002623077
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java:
##
@@ -157,7 +159,24 @@ public ProjectJoinTransposeRule(
@Value.Immutable(singleton = false)
public interface Config extends RelRule.Config {
Config DEFAULT = ImmutableProjectJoinTransposeRule.Config.builder()
-.withPreserveExprCondition(expr -> !(expr instanceof RexOver))
+.withPreserveExprCondition(expr -> {
+ // Non push down over's expression by default
Review Comment:
```suggestion
// Do not push down over's expression by default
```
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java:
##
@@ -157,7 +159,24 @@ public ProjectJoinTransposeRule(
@Value.Immutable(singleton = false)
public interface Config extends RelRule.Config {
Config DEFAULT = ImmutableProjectJoinTransposeRule.Config.builder()
-.withPreserveExprCondition(expr -> !(expr instanceof RexOver))
+.withPreserveExprCondition(expr -> {
+ // Non push down over's expression by default
+ if (expr instanceof RexOver) {
+return false;
+ }
+ final RelDataType relType = expr.getType();
+ if (SqlKind.CAST == expr.getKind()) {
+final RexCall castCall = (RexCall) expr;
+final RelDataType operand0Type =
castCall.getOperands().get(0).getType();
+if (relType.getSqlTypeName() == operand0Type.getSqlTypeName()
+&& relType.isNullable() != operand0Type.isNullable()) {
Review Comment:
casting `not null` to `nullable` could be pushed down?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] libenchao commented on a diff in pull request #183: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
libenchao commented on code in PR #183:
URL: https://github.com/apache/calcite-avatica/pull/183#discussion_r1002613948
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -222,15 +223,22 @@ public static T instantiatePlugin(Class
pluginClass,
final Class clazz = (Class) Class.forName(left);
final Field field;
field = clazz.getField(right);
-return pluginClass.cast(field.get(null));
+final Object fieldValue = field.get(null);
+if (fieldValue instanceof ThreadLocal) {
+ value = ((ThreadLocal) fieldValue).get();
+} else {
+ value = fieldValue;
+}
+return pluginClass.cast(value);
}
//noinspection unchecked
final Class clazz = (Class) Class.forName(className);
try {
// We assume that if there is an INSTANCE field it is static and
// has the right type.
final Field field = clazz.getField("INSTANCE");
-return pluginClass.cast(field.get(null));
+value = field.get(null);
+return pluginClass.cast(value);
Review Comment:
In this branch, `value` could also be a `ThreadLocal`?
##
core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java:
##
@@ -211,6 +211,7 @@ public static T instantiatePlugin(Class pluginClass,
String className) {
String right = null;
String left = null;
+Object value = null;
Review Comment:
Does`AvaticaUtils#instantiatePlugin` and `ConnectionConfigImpl#getPlugin`'s
java doc need to explicitly say that the 'INSTANCE' could be a `ThreadLocal`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] libenchao commented on pull request #2612: [CALCITE-4888] Unify type inferring logical for Sarg RexLiteral
libenchao commented on PR #2612: URL: https://github.com/apache/calcite/pull/2612#issuecomment-1287800206 I'm closing this PR because https://github.com/apache/calcite/pull/2853 has been merged and fix this issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao closed pull request #2612: [CALCITE-4888] Unify type inferring logical for Sarg RexLiteral
libenchao closed pull request #2612: [CALCITE-4888] Unify type inferring logical for Sarg RexLiteral URL: https://github.com/apache/calcite/pull/2612 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao closed pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
libenchao closed pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types URL: https://github.com/apache/calcite/pull/2853 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando closed pull request #2944: Fixed and improved javadoc instructions to regenerate RelOptRulesTest…
asolimando closed pull request #2944: Fixed and improved javadoc instructions to regenerate RelOptRulesTest… URL: https://github.com/apache/calcite/pull/2944 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2947: [MINOR] Add default catalog link to test fixtures
asolimando commented on PR #2947: URL: https://github.com/apache/calcite/pull/2947#issuecomment-1287672624 I have recently had the same issue, so +1 on the change. I noticed that the comments for the `sql()` method are all different for these three classes, maybe we can try to provide a single description, so that it's clear that the three methods are doing the same thing? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli opened a new pull request, #2947: [MINOR] Add default catalog link to test fixtures
JiajunBernoulli opened a new pull request, #2947: URL: https://github.com/apache/calcite/pull/2947 - When I write a unit test, I often see which tables and columns are available. At present, I have to search the catalog. If there is a link to jump directly, it is very convenient. - For new contributor, it will be easier to find the default catalog. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] henneberger commented on pull request #2909: Update SqlSelect to add hints to setOperand
henneberger commented on PR #2909: URL: https://github.com/apache/calcite/pull/2909#issuecomment-1287596760 Closing as invalid. This change would cause other issues. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] henneberger closed pull request #2909: Update SqlSelect to add hints to setOperand
henneberger closed pull request #2909: Update SqlSelect to add hints to setOperand URL: https://github.com/apache/calcite/pull/2909 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] HanumathRao commented on pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
HanumathRao commented on PR #2935: URL: https://github.com/apache/calcite/pull/2935#issuecomment-1287499498 > > I am using the call context to get the RelMetadataQuery, I think this is not fully formed during the matching of the rule > > @HanumathRao can you elaborate a bit what you mean when you say that the metadata query is not formed during `matches`? Both `matches` and `onMatch` take a `RelOptRuleCall` as parameter and in both cases `call.getMetadataQuery()` is available. Thanks @zabetak, I think I misunderstood your earlier comment. I thought that you are asking me to introduce a predicate in operand supplier itself to rule out the application of the rule. I fixed the code in the latest commit to return false for not applicable cases in matches method. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] HanumathRao commented on a diff in pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
HanumathRao commented on code in PR #2935:
URL: https://github.com/apache/calcite/pull/2935#discussion_r1002223506
##
testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java:
##
@@ -261,6 +261,14 @@ protected MockCatalogReaderSimple(RelDataTypeFactory
typeFactory,
productsTable.addColumn("SUPPLIERID", fixture.intType);
registerTable(productsTable);
+// Register "EMPTY_PRODUCTS" table.
+MockTable emptyProductsTable = MockTable.create(this, salesSchema,
"EMPTY_PRODUCTS",
+false, 2000D, 0.0);
Review Comment:
It was intentional but fixed it anyway in the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] HanumathRao commented on a diff in pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
HanumathRao commented on code in PR #2935: URL: https://github.com/apache/calcite/pull/2935#discussion_r1002223406 ## core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml: ## @@ -3058,6 +3058,50 @@ LogicalSort(sort0=[$7], dir0=[ASC], fetch=[0]) + + + + + + + + + + + + + + + + + + + + + +
[GitHub] [calcite] asolimando merged pull request #2901: [CALCITE-5264] Apply hint exclusion check for all rels in the RelOptCall
asolimando merged PR #2901: URL: https://github.com/apache/calcite/pull/2901 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] joshelser commented on a diff in pull request #184: CALCITE-5327 Make SSL key-store type configurable
joshelser commented on code in PR #184:
URL: https://github.com/apache/calcite-avatica/pull/184#discussion_r1001885363
##
server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java:
##
@@ -767,6 +771,27 @@ public Builder withTLS(File keystore, String
keystorePassword, File truststor
return this;
}
+/**
+ * Configures the server to use TLS for wire encryption.
+ *
+ * @param keystore The server's keystore
+ * @param keystorePassword The keystore's password
+ * @param truststore The truststore containing the key used to generate
the server's key
+ * @param truststorePassword The truststore's password
+ * @param keystoreType The keystore's type
+ * @return this
+ */
+public Builder withTLS(File keystore, String keystorePassword, File
truststore,
+ String truststorePassword, String keystoreType) {
+ this.usingTLS = true;
+ this.keystore = Objects.requireNonNull(keystore);
+ this.keystorePassword = Objects.requireNonNull(keystorePassword);
+ this.truststore = Objects.requireNonNull(truststore);
+ this.truststorePassword = Objects.requireNonNull(truststorePassword);
Review Comment:
```suggestion
this.withTLS(keystore, keystorePassword, truststore,
truststorePassword);
this.KeystoreType = Objects.requireNonNull(keystoreType);
```
##
server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java:
##
@@ -850,6 +875,9 @@ protected SslContextFactory.Server buildSSLContextFactory()
{
sslFactory.setKeyStorePassword(keystorePassword);
sslFactory.setTrustStorePath(truststore.getAbsolutePath());
sslFactory.setTrustStorePassword(truststorePassword);
+if (keystoreType != null &&
!keystoreType.equals(DEFAULT_KEYSTORE_TYPE)) {
+ sslFactory.setKeyStoreType(keystoreType);
Review Comment:
Could you add a comment here as to why this is important? Was there some
use-case which can accept a keystore in a different type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] Aitozi commented on pull request #2901: [CALCITE-5264] Apply hint exclusion check for all rels in the RelOptCall
Aitozi commented on PR #2901: URL: https://github.com/apache/calcite/pull/2901#issuecomment-1287071569 > @Aitozi, can you squash the commits into a single one having as commit message `[CALCITE-5264] HintStrategy rule exclusion does not match innermost rels`? As soon as this is done I am ready to merge the PR, thanks! Sure, done! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] rubenada merged pull request #182: [CALCITE-5257] NVARCHAR is treated as "UNKNOWN TYPE" when searching Oracle
rubenada merged PR #182: URL: https://github.com/apache/calcite-avatica/pull/182 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2901: [CALCITE-5264] Apply hint exclusion check for all rels in the RelOptCall
asolimando commented on PR #2901: URL: https://github.com/apache/calcite/pull/2901#issuecomment-1286888938 @Aitozi, can you squash the commits into a single one having as commit message `[CALCITE-5264] HintStrategy rule exclusion does not match innermost rels`? As soon as this is done I am ready to merge the PR, thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] rubenada merged pull request #163: [CALCITE-4900] NullPointerException when send ExectuteRequest via protobuf with no parameters
rubenada merged PR #163: URL: https://github.com/apache/calcite-avatica/pull/163 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] dssysolyatin commented on a diff in pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
dssysolyatin commented on code in PR #2819:
URL: https://github.com/apache/calcite/pull/2819#discussion_r1001563447
##
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##
@@ -721,4 +738,31 @@ boolean canImplicitTypeCast(List types,
List familie
}
return null;
}
+
+ /**
+ * Coerce STRING type to ARRAY type.
+ */
+ protected Boolean coerceStringToArray(
+ SqlCall call,
+ SqlNode operand,
+ int index,
+ RelDataType fromType,
+ RelDataType targetType) {
+if (validator.config().conformance().allowCoercionStringToArray()
+&& SqlTypeUtil.isString(fromType)
+&& SqlTypeUtil.isArray(targetType)
+&& operand instanceof SqlCharStringLiteral
+) {
+ try {
+SqlNode arrayValue = SqlParserUtil.parseArrayLiteral(
+((SqlCharStringLiteral) operand).getValueAs(String.class));
+call.setOperand(index, arrayValue);
+updateInferredType(arrayValue, targetType);
+ } catch (Throwable e) {
+return false;
Review Comment:
I managed to replace throwable by SqlParseException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] dssysolyatin commented on a diff in pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
dssysolyatin commented on code in PR #2819:
URL: https://github.com/apache/calcite/pull/2819#discussion_r1001561672
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -243,6 +243,17 @@ public class ${parser.class} extends SqlAbstractParserImpl
return SqlStmtList();
}
+public SqlNode parseArray() throws SqlParseException {
+switchTo(LexicalState.BQID);
+try {
+ return ArrayLiteral();
+} catch (ParseException ex) {
Review Comment:
I used catch twice because javaCC can not parse `catch (ParseException |
TokenMsgError ex)` for some reason.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] richardantal commented on pull request #184: CALCITE-5327 Make SSL key-store type configurable
richardantal commented on PR #184: URL: https://github.com/apache/calcite-avatica/pull/184#issuecomment-1286639811 I am not able to add reviewers for this PR, but could you please take a look at it? @joshelser @julianhyde @stoty -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] dependabot[bot] closed pull request #2817: Bump nokogiri from 1.13.4 to 1.13.6 in /site
dependabot[bot] closed pull request #2817: Bump nokogiri from 1.13.4 to 1.13.6 in /site URL: https://github.com/apache/calcite/pull/2817 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] dependabot[bot] commented on pull request #2817: Bump nokogiri from 1.13.4 to 1.13.6 in /site
dependabot[bot] commented on PR #2817: URL: https://github.com/apache/calcite/pull/2817#issuecomment-1286379401 Superseded by #2945. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] dependabot[bot] opened a new pull request, #2945: Bump nokogiri from 1.13.4 to 1.13.9 in /site
dependabot[bot] opened a new pull request, #2945: URL: https://github.com/apache/calcite/pull/2945 Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.13.4 to 1.13.9. Release notes Sourced from https://github.com/sparklemotion/nokogiri/releases;>nokogiri's releases. 1.13.9 / 2022-10-18 Security [CRuby] Vendored libxml2 is updated to address https://nvd.nist.gov/vuln/detail/CVE-2022-2309;>CVE-2022-2309, https://nvd.nist.gov/vuln/detail/CVE-2022-40304;>CVE-2022-40304, and https://nvd.nist.gov/vuln/detail/CVE-2022-40303;>CVE-2022-40303. See https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-2qc6-mcvw-92cw;>GHSA-2qc6-mcvw-92cw for more information. [CRuby] Vendored zlib is updated to address https://ubuntu.com/security/CVE-2022-37434;>CVE-2022-37434. Nokogiri was not affected by this vulnerability, but this version of zlib was being flagged up by some vulnerability scanners, see https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2626;>#2626 for more information. Dependencies [CRuby] Vendored libxml2 is updated to https://gitlab.gnome.org/GNOME/libxml2/-/releases/v2.10.3;>v2.10.3 from v2.9.14. [CRuby] Vendored libxslt is updated to https://gitlab.gnome.org/GNOME/libxslt/-/releases/v1.1.37;>v1.1.37 from v1.1.35. [CRuby] Vendored zlib is updated from 1.2.12 to 1.2.13. (See https://github.com/sparklemotion/nokogiri/blob/v1.13.x/LICENSE-DEPENDENCIES.md#platform-releases;>LICENSE-DEPENDENCIES.md for details on which packages redistribute this library.) Fixed [CRuby] Nokogiri::XML::Namespace objects, when compacted, update their internal struct's reference to the Ruby object wrapper. Previously, with GC compaction enabled, a segmentation fault was possible after compaction was triggered. [https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2658;>#2658] (Thanks, https://github.com/eightbitraptor;>@​eightbitraptor and https://github.com/peterzhu2118;>@​peterzhu2118!) [CRuby] Document#remove_namespaces! now defers freeing the underlying xmlNs struct until the Document is GCed. Previously, maintaining a reference to a Namespace object that was removed in this way could lead to a segfault. [https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2658;>#2658] sha256 checksums: 9b69829561d30c4461ea803baeaf3460e8b145cff7a26ce397119577a4083a02 nokogiri-1.13.9-aarch64-linux.gem e76ebb4b7b2e02c72b2d1541289f8b0679fb5984867cf199d89b8ef485764956 nokogiri-1.13.9-arm64-darwin.gem 15bae7d08bddeaa898d8e3f558723300137c26a2dc2632a1f89c8574c4467165 nokogiri-1.13.9-java.gem f6a1dbc7229184357f3129503530af73cc59ceba4932c700a458a561edbe04b9 nokogiri-1.13.9-x64-mingw-ucrt.gem 36d935d799baa4dc488024f71881ff0bc8b172cecdfc54781169c40ec02cbdb3 nokogiri-1.13.9-x64-mingw32.gem ebaf82aa9a11b8fafb67873d19ee48efb565040f04c898cdce8ca0cd53ff1a12 nokogiri-1.13.9-x86-linux.gem 11789a2a11b28bc028ee111f23311461104d8c4468d5b901ab7536b282504154 nokogiri-1.13.9-x86-mingw32.gem 01830e1646803ff91c0fe94bc768ff40082c6de8cfa563dafd01b3f7d5f9d795 nokogiri-1.13.9-x86_64-darwin.gem 8e93b8adec22958013799c8690d81c2cdf8a90b6f6e8150ab22e11895844d781 nokogiri-1.13.9-x86_64-linux.gem 96f37c1baf0234d3ae54c2c89aef7220d4a8a1b03d2675ff7723565b0a095531 nokogiri-1.13.9.gem 1.13.8 / 2022-07-23 Deprecated XML::Reader#attribute_nodes is deprecated due to incompatibility between libxml2's xmlReader memory semantics and Ruby's garbage collector. Although this method continues to exist for backwards compatibility, it is unsafe to call and may segfault. This method will be removed in a future version of Nokogiri, and callers should use #attribute_hash instead. [https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2598;>#2598] Improvements XML::Reader#attribute_hash is a new method to safely retrieve the attributes of a node from XML::Reader. [https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2598;>#2598, https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2599;>#2599] Fixed ... (truncated) Changelog Sourced from https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md;>nokogiri's changelog. 1.13.9 / 2022-10-18 Security [CRuby] Vendored libxml2 is updated to address https://nvd.nist.gov/vuln/detail/CVE-2022-2309;>CVE-2022-2309, https://nvd.nist.gov/vuln/detail/CVE-2022-40304;>CVE-2022-40304, and https://nvd.nist.gov/vuln/detail/CVE-2022-40303;>CVE-2022-40303. See https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-2qc6-mcvw-92cw;>GHSA-2qc6-mcvw-92cw for more information. [CRuby] Vendored zlib is updated to address https://ubuntu.com/security/CVE-2022-37434;>CVE-2022-37434. Nokogiri was not affected by this vulnerability, but this version of zlib was being flagged up by some vulnerability
[GitHub] [calcite] julianhyde closed pull request #2943: [CALCITE-5339] Use Method#getParameterCount rather than Method#getParameterTypes to get length
julianhyde closed pull request #2943: [CALCITE-5339] Use Method#getParameterCount rather than Method#getParameterTypes to get length URL: https://github.com/apache/calcite/pull/2943 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
julianhyde commented on code in PR #2819:
URL: https://github.com/apache/calcite/pull/2819#discussion_r1000994647
##
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##
@@ -721,4 +738,31 @@ boolean canImplicitTypeCast(List types,
List familie
}
return null;
}
+
+ /**
+ * Coerce STRING type to ARRAY type.
+ */
+ protected Boolean coerceStringToArray(
+ SqlCall call,
+ SqlNode operand,
+ int index,
+ RelDataType fromType,
+ RelDataType targetType) {
+if (validator.config().conformance().allowCoercionStringToArray()
+&& SqlTypeUtil.isString(fromType)
+&& SqlTypeUtil.isArray(targetType)
+&& operand instanceof SqlCharStringLiteral
+) {
+ try {
+SqlNode arrayValue = SqlParserUtil.parseArrayLiteral(
+((SqlCharStringLiteral) operand).getValueAs(String.class));
+call.setOperand(index, arrayValue);
+updateInferredType(arrayValue, targetType);
+ } catch (Throwable e) {
+return false;
Review Comment:
Yes, `catch Error` is just as bad as `catch Throwable`.
Is it possible for `parseArrayLiteral` to declare a short list of exceptions
that it may throw? Some of them will be user errors (i.e. the string is not
valid format); internal errors (e.g. `NullPointerException`,
`ArrayIndexOutOfBoundsException`) should be rethrown, not become `return false`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando opened a new pull request, #2944: Fixed and improved javadoc instructions to regenerate RelOptRulesTest…
asolimando opened a new pull request, #2944: URL: https://github.com/apache/calcite/pull/2944 ….xml, updated RelOptRulesTest.xml to match current tests in RelOptRulesTest.java -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] dssysolyatin commented on a diff in pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
dssysolyatin commented on code in PR #2819:
URL: https://github.com/apache/calcite/pull/2819#discussion_r1000675480
##
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##
@@ -721,4 +738,31 @@ boolean canImplicitTypeCast(List types,
List familie
}
return null;
}
+
+ /**
+ * Coerce STRING type to ARRAY type.
+ */
+ protected Boolean coerceStringToArray(
+ SqlCall call,
+ SqlNode operand,
+ int index,
+ RelDataType fromType,
+ RelDataType targetType) {
+if (validator.config().conformance().allowCoercionStringToArray()
+&& SqlTypeUtil.isString(fromType)
+&& SqlTypeUtil.isArray(targetType)
+&& operand instanceof SqlCharStringLiteral
+) {
+ try {
+SqlNode arrayValue = SqlParserUtil.parseArrayLiteral(
+((SqlCharStringLiteral) operand).getValueAs(String.class));
+call.setOperand(index, arrayValue);
+updateInferredType(arrayValue, targetType);
+ } catch (Throwable e) {
+return false;
Review Comment:
There is a problem that parseArrayLiteral can throw any exception. It
depends on parser logic. I replaced Throwable by `Exception | Error` but it's
not much better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando merged pull request #2941: Updated javadoc instructions to regenerate RelOptRulesTest.xml replac…
asolimando merged PR #2941: URL: https://github.com/apache/calcite/pull/2941 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] ILuffZhe commented on pull request #182: [CALCITE-5257] NVARCHAR is treated as "UNKNOWN TYPE" when searching Oracle
ILuffZhe commented on PR #182: URL: https://github.com/apache/calcite-avatica/pull/182#issuecomment-1284743905 Thanks for your review, @rubenada I've squashed the commits. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] snuyanzin opened a new pull request, #2943: [CALCITE-5339] Use Method#getParameterCount rather than Method#getParameterTypes to get length
snuyanzin opened a new pull request, #2943: URL: https://github.com/apache/calcite/pull/2943 As stated in jira issue `Method#getParameterTypes` each time creates a new array so it's better to use `Method#getParameterCount` to know length of array -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde closed pull request #2926: [CALCITE-5305] Support string constants with c-style escapes
julianhyde closed pull request #2926: [CALCITE-5305] Support string constants with c-style escapes URL: https://github.com/apache/calcite/pull/2926 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando opened a new pull request, #2941: Updated javadoc instructions to regenerate RelOptRulesTest.xml replac…
asolimando opened a new pull request, #2941: URL: https://github.com/apache/calcite/pull/2941 …ing Maven path with Gradle path -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] rubenada commented on pull request #182: [CALCITE-5257] NVARCHAR is treated as "UNKNOWN TYPE" when searching Oracle
rubenada commented on PR #182: URL: https://github.com/apache/calcite-avatica/pull/182#issuecomment-1284238772 Thanks @ILuffZhe , lgtm. Could you please squash commits into a single one? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
libenchao commented on PR #2853: URL: https://github.com/apache/calcite/pull/2853#issuecomment-1283915022 @abhishek-das-gupta I found that `RexBuilder#makeBetween` also may have the same problem, could you help to confirm this, and fix it if it exists? (In this issue, or a follow-up issue, it's up to you, I'm ok with both way) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao closed pull request #2936: [CALCITE-5326] fix values from sql merge
libenchao closed pull request #2936: [CALCITE-5326] fix values from sql merge URL: https://github.com/apache/calcite/pull/2936 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on a diff in pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
zabetak commented on code in PR #2935: URL: https://github.com/apache/calcite/pull/2935#discussion_r999314056 ## core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml: ## @@ -3058,6 +3058,50 @@ LogicalSort(sort0=[$7], dir0=[ASC], fetch=[0]) + + + + + + + + + + + + + + + + + + + + + +
[GitHub] [calcite] zabetak commented on a diff in pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
zabetak commented on code in PR #2935:
URL: https://github.com/apache/calcite/pull/2935#discussion_r999312483
##
testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java:
##
@@ -261,6 +261,14 @@ protected MockCatalogReaderSimple(RelDataTypeFactory
typeFactory,
productsTable.addColumn("SUPPLIERID", fixture.intType);
registerTable(productsTable);
+// Register "EMPTY_PRODUCTS" table.
+MockTable emptyProductsTable = MockTable.create(this, salesSchema,
"EMPTY_PRODUCTS",
+false, 2000D, 0.0);
Review Comment:
Is it intentional to have rows > masRows?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
zabetak commented on PR #2935: URL: https://github.com/apache/calcite/pull/2935#issuecomment-1283786273 > I am using the call context to get the RelMetadataQuery, I think this is not fully formed during the matching of the rule @HanumathRao can you elaborate a bit what you mean when you say that the metadata query is not formed during `matches`? Both `matches` and `onMatch` take a `RelOptRuleCall` as parameter and in both cases `call.getMetadataQuery()` is available. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] wojustme commented on pull request #2686: [CALCITE-4982] NonNull field is pushed down into leaf of outer-join in 'ProjectJoinTransposeRule'
wojustme commented on PR #2686: URL: https://github.com/apache/calcite/pull/2686#issuecomment-1283656880 Add default config: PreserveExprCondition Shouldn't push non-null field, which is wrapped by CAST's operator. Hi @libenchao Please review this pr again, thanks a lot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando merged pull request #2940: [CALCITE-5337] UnionPullUpConstantsRule produces an invalid plan when pulling up constants for nullable fields
asolimando merged PR #2940: URL: https://github.com/apache/calcite/pull/2940 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2940: [CALCITE-5337] UnionPullUpConstantsRule produces an invalid plan when pulling up constants for nullable fields
asolimando commented on PR #2940: URL: https://github.com/apache/calcite/pull/2940#issuecomment-1283648291 Test failure after squashing is caused by the known flacky test `org.apache.calcite.test.ServerTest`, not related to the PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2901: [CALCITE-5264] Apply hint exclusion check for all rels in the RelOptCall
asolimando commented on PR #2901: URL: https://github.com/apache/calcite/pull/2901#issuecomment-1282762416 The PR LGTM, I will merge by the end of this week if there will be no objections -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] abhishek-das-gupta commented on a diff in pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
abhishek-das-gupta commented on code in PR #2853:
URL: https://github.com/apache/calcite/pull/2853#discussion_r997862622
##
core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java:
##
@@ -624,6 +624,29 @@ private void checkDate(RexLiteral literal) {
assertThat(inCall.getKind(), is(SqlKind.SEARCH));
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final RelDataTypeFactory typeFactory = new
SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RexBuilder rexBuilder = new RexBuilder(typeFactory);
+final RelDataType decimalType =
typeFactory.createSqlType(SqlTypeName.DECIMAL);
+RexNode left = rexBuilder.makeInputRef(decimalType, 0);
+final RexNode literal1 = rexBuilder.makeExactLiteral(new
BigDecimal("1.0"));
+final RexNode literal2 = rexBuilder.makeExactLiteral(new
BigDecimal("2.0"));
+
+RexNode inCall = rexBuilder.makeIn(left, ImmutableList.of(literal1,
literal2));
+assertThat(inCall.getKind(), is(SqlKind.SEARCH));
+
+final RexNode sarg = ((RexCall) inCall).operands.get(1);
+final RelDataType leastRestrictiveType =
+typeFactory.leastRestrictive(ImmutableList.of(literal1.getType(),
literal2.getType()));
Review Comment:
Yes, i think I mistinterpreted it. Apologies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] wojustme opened a new pull request, #2939: The rule of AGGREGATE_EXPAND_DISTINCT_AGGREGATES_TO_JOIN throws error, when meeting the rollup's query
wojustme opened a new pull request, #2939: URL: https://github.com/apache/calcite/pull/2939 ISSUE: https://issues.apache.org/jira/browse/CALCITE-5328 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] ILuffZhe commented on pull request #182: [CALCITE-5257] NVARCHAR is treated as "UNKNOWN TYPE" when searching Oracle
ILuffZhe commented on PR #182: URL: https://github.com/apache/calcite-avatica/pull/182#issuecomment-1281717165 @rubenada Hi, Ruben. I've added unit test for this case, please take a look when you have time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
libenchao commented on code in PR #2853:
URL: https://github.com/apache/calcite/pull/2853#discussion_r997163233
##
core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java:
##
@@ -624,6 +624,29 @@ private void checkDate(RexLiteral literal) {
assertThat(inCall.getKind(), is(SqlKind.SEARCH));
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final RelDataTypeFactory typeFactory = new
SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RexBuilder rexBuilder = new RexBuilder(typeFactory);
+final RelDataType decimalType =
typeFactory.createSqlType(SqlTypeName.DECIMAL);
+RexNode left = rexBuilder.makeInputRef(decimalType, 0);
+final RexNode literal1 = rexBuilder.makeExactLiteral(new
BigDecimal("1.0"));
+final RexNode literal2 = rexBuilder.makeExactLiteral(new
BigDecimal("2.0"));
+
+RexNode inCall = rexBuilder.makeIn(left, ImmutableList.of(literal1,
literal2));
+assertThat(inCall.getKind(), is(SqlKind.SEARCH));
+
+final RexNode sarg = ((RexCall) inCall).operands.get(1);
+final RelDataType leastRestrictiveType =
+typeFactory.leastRestrictive(ImmutableList.of(literal1.getType(),
literal2.getType()));
Review Comment:
I'm not sure your new commit addressed Julian's comment, but per my
understanding, I prefer the below way (and this is how I understand Julian's
comment), what do you think?
```java
RelDataType expected = typeFactory.createSqlType(SqlTypeName.DECIMAL, 6,
1);
assertEquals(sarg.getType(), expected);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] rubenada commented on pull request #182: [CALCITE-5257] NVARCHAR is treated as "UNKNOWN TYPE" when searching Oracle
rubenada commented on PR #182: URL: https://github.com/apache/calcite-avatica/pull/182#issuecomment-1280950195 @ILuffZhe could you please add a unit test for the problem? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] rubenada commented on pull request #163: [CALCITE-4900] NullPointerException when send ExectuteRequest via protobuf with no parameters
rubenada commented on PR #163: URL: https://github.com/apache/calcite-avatica/pull/163#issuecomment-1280946552 Thanks @Logioniz for the PR. @NobiGo the PR seems in a good shape. Do you have any further comments? Otherwise I'll merge it in the coming days. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
rubenada commented on PR #2937: URL: https://github.com/apache/calcite/pull/2937#issuecomment-1280924430 @zabetak as you already mentioned in Jira, it seems PruneEmptyRule is the only one that lacks a default config, so we should definitely add it and align with the rest (see e.g. `ReduceExpressionsRule` and subclasses, which seems similar case, and which does define default configs). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996677165
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
IMHO, SET_QUERY should not be a EXPRESSION.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996676210
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
So what's your suggestion on `SqlWriter#inQuery`'s java doc?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996675145
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -2241,6 +2241,34 @@ void checkPeriodPredicate(Checker checker) {
sql(sql).fails("(?s)Encountered \"with\" at .*");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithSelect() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithOrderBy() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2 order by deptno\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`\n"
++ "ORDER BY `DEPTNO`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
Review Comment:
OK, i will change them back on next commits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996611975
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
|| (frame.frameType == FrameTypeEnum.SETOP);
}
+ @Override public boolean inWithBody() {
+return frame != null && frame.frameType == FrameTypeEnum.WITH_BODY;
Review Comment:
Sorry about this one, I thought it was `frame.frameType != null`
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -2241,6 +2241,34 @@ void checkPeriodPredicate(Checker checker) {
sql(sql).fails("(?s)Encountered \"with\" at .*");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithSelect() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithOrderBy() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2 order by deptno\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`\n"
++ "ORDER BY `DEPTNO`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
Review Comment:
IMHO, we'd better to not comment other test cases to this issue cause it may
be added for other purpose.
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
Maybe I'm not clear on this. I'm not saying that is should be changed from
"a query context" to "a sub-query context".
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
`SqlKind.EXPRESSION` uses `EnumSet.complementOf`, hence its document and its
result is consistent. The question is "should SET_QUERY be a EXPRESSION"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996599562
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
Yes, it's very strange that `SqlKind.EXPRESSION` contains UNION, and
according to declaration of `SqlKind.EXPRESSION`, it should contains
`SELECT/ORDER_BY/WITH/JOIN` etc, and not contains `UNION`, but when i debug,
something is different, you can not find `SELECT/ORDER_BY/WITH/JOIN` in
`SqlKind.EXPRESSION`, and `UNION` will appear. Let's back to the topic, might
putting `inWithBody()` into `SqlWriter#isAlwaysUseParentheses ` will be better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996593733
##
core/src/main/java/org/apache/calcite/sql/SqlWith.java:
##
@@ -103,8 +103,9 @@ private SqlWithOperator() {
}
writer.endList(frame1);
final SqlWriter.Frame frame2 =
- writer.startList(SqlWriter.FrameTypeEnum.SIMPLE);
- with.body.unparse(writer, 100, 100);
+ writer.startList(SqlWriter.FrameTypeEnum.WITH_BODY);
+ // leftPrec: 2, rightPrec: 3 are the low bound to WITH operator
+ with.body.unparse(writer, 2, 3);
Review Comment:
You are right, i agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996593539
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -2241,6 +2241,34 @@ void checkPeriodPredicate(Checker checker) {
sql(sql).fails("(?s)Encountered \"with\" at .*");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithSelect() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithOrderBy() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2 order by deptno\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`\n"
++ "ORDER BY `DEPTNO`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
Review Comment:
I just want to show these two cases should be related to this issue, maybe i
should not change past cases? if so, i will change them back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996592477
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
|| (frame.frameType == FrameTypeEnum.SETOP);
}
+ @Override public boolean inWithBody() {
+return frame != null && frame.frameType == FrameTypeEnum.WITH_BODY;
Review Comment:
Maybe not, NPE will be thrown on some unit tests, e.g.
`SqlParserTest#testNewSpecification`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996591355
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
`SqlWriter#inQuery` 's java doc says: "whether we are currently in a query
context", not "whether we are currently in a sub-query context", so I think
current java doc is appropriate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996591355
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
SqlWriter#inQuery 's java doc says: "whether we are currently in a query
context", not "whether we are currently in a sub-query context", so I think
current java doc is appropriate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei commented on pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei commented on PR #2938: URL: https://github.com/apache/calcite/pull/2938#issuecomment-1280237370 > OH~, thank you very much for your tip, i have already changed the email in my local git config. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r996443682
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
|| (frame.frameType == FrameTypeEnum.SETOP);
}
+ @Override public boolean inWithBody() {
+return frame != null && frame.frameType == FrameTypeEnum.WITH_BODY;
Review Comment:
`frame != null` can be removed.
##
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean
windowDeclListNewline) {
return (frame == null)
|| (frame.frameType == FrameTypeEnum.SELECT)
|| (frame.frameType == FrameTypeEnum.ORDER_BY)
-|| (frame.frameType == FrameTypeEnum.WITH)
+|| (frame.frameType == FrameTypeEnum.WITH_BODY)
Review Comment:
The `SqlWriter#inQuery`'s java doc should also be updated. IIUC,
`SqlWriter#inQuery` is for deciding whether a SELECT is a sub-query. Hence the
`inQuery` actually means that "a select is a top-most query". But now, the
`inQuery`'s meaning has expanded, we may need to reflect it in the java doc,
and may change it's name to a more proper one in the future.
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -2241,6 +2241,34 @@ void checkPeriodPredicate(Checker checker) {
sql(sql).fails("(?s)Encountered \"with\" at .*");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithSelect() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
+ @Test void testWithOrderBy() {
+final String sql = "with emp2 as (select * from emp)\n"
++ "select * from emp2 order by deptno\n";
+final String expected = "WITH `EMP2` AS (SELECT *\n"
++ "FROM `EMP`) SELECT *\n"
++ "FROM `EMP2`\n"
++ "ORDER BY `DEPTNO`";
+sql(sql).ok(expected);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5299;>[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH
body. */
Review Comment:
Why the existing `testWithNestedInSubQuery`/`testWithUnion` is also a test
case for CALCITE-5299
##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -118,7 +118,7 @@ public int operandCount() {
final SqlDialect dialect = writer.getDialect();
if (leftPrec > operator.getLeftPrec()
|| (operator.getRightPrec() <= rightPrec && (rightPrec != 0))
-|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION)) {
+|| writer.isAlwaysUseParentheses() && isA(SqlKind.EXPRESSION) &&
!writer.inWithBody()) {
Review Comment:
Why must you add this condition? I know `testWithUnion` will fail without
this, however, it's surprising that `SqlKind.SET_QUERY` is a
`SqlKind.EXPRESSION` while `SELECT`/`ORDER_BY`/`WITH`/`JOIN` etc. are not.
##
core/src/main/java/org/apache/calcite/sql/SqlWith.java:
##
@@ -103,8 +103,9 @@ private SqlWithOperator() {
}
writer.endList(frame1);
final SqlWriter.Frame frame2 =
- writer.startList(SqlWriter.FrameTypeEnum.SIMPLE);
- with.body.unparse(writer, 100, 100);
+ writer.startList(SqlWriter.FrameTypeEnum.WITH_BODY);
+ // leftPrec: 2, rightPrec: 3 are the low bound to WITH operator
+ with.body.unparse(writer, 2, 3);
Review Comment:
Hard coded `2` and `3` can be replaced with `getLeftPrec()` and
`getRightPrec()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] abhishek-das-gupta commented on a diff in pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
abhishek-das-gupta commented on code in PR #2853:
URL: https://github.com/apache/calcite/pull/2853#discussion_r996345727
##
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##
@@ -1258,6 +1259,26 @@ private static String toSql(RelNode root, SqlDialect
dialect,
relFn(relFn).optimize(rules, null).ok(expected);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final Function relFn = b -> b
+.scan("EMP")
+.filter(
+b.or(b.isNull(b.field("COMM")),
+ b.in(
+ b.field("COMM"),
+ b.literal(new BigDecimal("1.0")), b.literal(new
BigDecimal("2.0")))
+)
+)
Review Comment:
Sorry about this. Addressed this.
##
core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java:
##
@@ -624,6 +624,29 @@ private void checkDate(RexLiteral literal) {
assertThat(inCall.getKind(), is(SqlKind.SEARCH));
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final RelDataTypeFactory typeFactory = new
SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RexBuilder rexBuilder = new RexBuilder(typeFactory);
+final RelDataType decimalType =
typeFactory.createSqlType(SqlTypeName.DECIMAL);
+RexNode left = rexBuilder.makeInputRef(decimalType, 0);
+final RexNode literal1 = rexBuilder.makeExactLiteral(new
BigDecimal("1.0"));
+final RexNode literal2 = rexBuilder.makeExactLiteral(new
BigDecimal("2.0"));
+
+RexNode inCall = rexBuilder.makeIn(left, ImmutableList.of(literal1,
literal2));
+assertThat(inCall.getKind(), is(SqlKind.SEARCH));
+
+final RexNode sarg = ((RexCall) inCall).operands.get(1);
+final RelDataType leastRestrictiveType =
+typeFactory.leastRestrictive(ImmutableList.of(literal1.getType(),
literal2.getType()));
Review Comment:
ACK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] HanumathRao commented on pull request #2935: [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata
HanumathRao commented on PR #2935: URL: https://github.com/apache/calcite/pull/2935#issuecomment-1279557853 Thanks @zabetak for the review. I have addressed all the comments except the following. _It is better to put the row count check in the matches method. When the condition is not satisfied the rule can be eliminated much earlier._ I am using the call context to get the RelMetadataQuery, I think this is not fully formed during the matching of the rule. I thought about using the MetadataQuery singleton INSTANCE, but it can get complex during the matching. Please let me know if you have an easier way in mind. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
julianhyde commented on code in PR #2853:
URL: https://github.com/apache/calcite/pull/2853#discussion_r996032708
##
core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java:
##
@@ -624,6 +624,29 @@ private void checkDate(RexLiteral literal) {
assertThat(inCall.getKind(), is(SqlKind.SEARCH));
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final RelDataTypeFactory typeFactory = new
SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RexBuilder rexBuilder = new RexBuilder(typeFactory);
+final RelDataType decimalType =
typeFactory.createSqlType(SqlTypeName.DECIMAL);
+RexNode left = rexBuilder.makeInputRef(decimalType, 0);
+final RexNode literal1 = rexBuilder.makeExactLiteral(new
BigDecimal("1.0"));
+final RexNode literal2 = rexBuilder.makeExactLiteral(new
BigDecimal("2.0"));
+
+RexNode inCall = rexBuilder.makeIn(left, ImmutableList.of(literal1,
literal2));
+assertThat(inCall.getKind(), is(SqlKind.SEARCH));
+
+final RexNode sarg = ((RexCall) inCall).operands.get(1);
+final RelDataType leastRestrictiveType =
+typeFactory.leastRestrictive(ImmutableList.of(literal1.getType(),
literal2.getType()));
Review Comment:
Can you assert what the least restrictive type is? It will make the test
more concrete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #2853: [CALCITE-4632] SARG datatype should be less restictive than any of the input types
julianhyde commented on code in PR #2853:
URL: https://github.com/apache/calcite/pull/2853#discussion_r996031793
##
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##
@@ -1258,6 +1259,26 @@ private static String toSql(RelNode root, SqlDialect
dialect,
relFn(relFn).optimize(rules, null).ok(expected);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-4632;>[CALCITE-4632]
+ * Find the least restrictive datatype for SARG. */
+ @Test void testLeastRestrictiveTypeForSarg() {
+final Function relFn = b -> b
+.scan("EMP")
+.filter(
+b.or(b.isNull(b.field("COMM")),
+ b.in(
+ b.field("COMM"),
+ b.literal(new BigDecimal("1.0")), b.literal(new
BigDecimal("2.0")))
+)
+)
Review Comment:
Please remove newlines before close-parentheses. This is not Calcite's
coding style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] l4wei opened a new pull request, #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
l4wei opened a new pull request, #2938: URL: https://github.com/apache/calcite/pull/2938 Remove unnecessary parentheses around SELECT in WITH body -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] l4wei closed pull request #2923: [CALCITE-5252] Unparsing "WITH ... AS (SELECT ... UNION SELECT ...)" missing parentheses
l4wei closed pull request #2923: [CALCITE-5252] Unparsing "WITH ... AS (SELECT ... UNION SELECT ...)" missing parentheses URL: https://github.com/apache/calcite/pull/2923 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
zabetak commented on PR #2937: URL: https://github.com/apache/calcite/pull/2937#issuecomment-1278688528 @rubenada I guess there are other rules which miss a `DEFAULT` config instance. The problem though may not be so serious in every case if rules are not declared as 'RelOptRule` but as a subclass of `RelRule`. Maybe it makes sense to do this refactoring for every rule to keep things uniform. WDYT? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
rubenada commented on PR #2937: URL: https://github.com/apache/calcite/pull/2937#issuecomment-1277715384 @zabetak IMO the change makes sense, lgtm. Have you checked if there are other rules that could benefit from the same refactoring? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
