Re: [PR] [CALCITE-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
sonarcloud[bot] commented on PR #3663: URL: https://github.com/apache/calcite/pull/3663#issuecomment-2065453164 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3663) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3663=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3663=false=true)  [70.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3663) -- 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
Re: [PR] [CALCITE-3522] Sql validator limits decimal literals to 64 bits and [CALCITE-6169] EnumUtils.convert does not implement the correct SQL cast semantics [calcite]
mihaibudiu commented on PR #3589: URL: https://github.com/apache/calcite/pull/3589#issuecomment-2065329884 I have submitted https://github.com/apache/calcite/pull/3764, which provides just a fix for [CALCITE-6169]. Once that is merged, I will resubmit a much smaller PR here to fix separately [CALCITE-3522] -- 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
Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
mihaibudiu commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1571322599
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -559,15 +558,6 @@ static SqlOperatorTable operatorTableFor(SqlLibrary
library) {
}
@Test void testArithmeticOperatorsFails() {
-expr("^power(2,'abc')^")
Review Comment:
Yes, see the operatorTableFor() function.
--
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
Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1571319246
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -559,15 +558,6 @@ static SqlOperatorTable operatorTableFor(SqlLibrary
library) {
}
@Test void testArithmeticOperatorsFails() {
-expr("^power(2,'abc')^")
Review Comment:
The `POWER` function is no longer available in `SqlStdOperatorTable`. The
implementation to use depends on which library is active since PostgreSQL can
use a different return type.
This test fails as it was since `POWER` is not in `StdOperatorTable`. Is
there a way to specify the library to use for the test?
--
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
Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1571317238
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -773,6 +791,28 @@ public static SqlCall stripSeparator(SqlCall call) {
return null;
};
+ public static final SqlReturnTypeInference DECIMAL_OR_DOUBLE = opBinding -> {
Review Comment:
Removed `DOUBLE_OR_DECIMAL`.
--
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
Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1571317026
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -410,6 +410,24 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference DOUBLE_FORCE_NULLABLE =
DOUBLE.andThen(SqlTypeTransforms.FORCE_NULLABLE);
+ /**
Review Comment:
Moved the comment to `DECIMAL_OR_DOUBLE`. Removed `DOUBLE_OR_DECIMAL`.
--
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
Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1571315674
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -2216,7 +2216,39 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
@LibraryOperator(libraries = {BIG_QUERY, SPARK})
public static final SqlFunction POW =
- SqlStdOperatorTable.POWER.withName("POW");
+ SqlBasicFunction.create("POW",
+ ReturnTypes.DOUBLE_NULLABLE,
+ OperandTypes.NUMERIC_NUMERIC,
+ SqlFunctionCategory.NUMERIC);
+
+ /** The {@code POWER(numeric, numeric)} function.
+ *
+ * The return type is always {@code DOUBLE} since we don't know
+ * what the result type will be by just looking at the operand types. For
+ * example {@code POWER(INTEGER, INTEGER)} can return a non-INTEGER if the
+ * second operand is negative.
+ */
+ @LibraryOperator(libraries = { BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL,
ORACLE, SNOWFLAKE,
+ SPARK })
+ public static final SqlFunction POWER =
+ SqlBasicFunction.create("POWER",
+ ReturnTypes.DOUBLE_NULLABLE,
+ OperandTypes.NUMERIC_NUMERIC,
+ SqlFunctionCategory.NUMERIC);
+
+ /** The {@code POWER(numeric, numeric)} function.
+ *
+ * The return type is always {@code DOUBLE} since we don't know
Review Comment:
Updated the comment.
--
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
Re: [PR] [CALCITE-6340] RelBuilder drops set conventions when aggregating over duplicate projected fields [calcite]
sonarcloud[bot] commented on PR #3757: URL: https://github.com/apache/calcite/pull/3757#issuecomment-2065035587 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3757) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3757=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite=3757=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3757=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3757=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3757=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3757) -- 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
Re: [PR] [CALCITE-6322] Casts to DECIMAL types are ignored [calcite]
sonarcloud[bot] commented on PR #3733: URL: https://github.com/apache/calcite/pull/3733#issuecomment-2064943328 Please retry analysis of this Pull-Request directly on SonarCloud -- 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
Re: [PR] [CALCITE-6322] Casts to DECIMAL types are ignored [calcite]
sonarcloud[bot] commented on PR #3733: URL: https://github.com/apache/calcite/pull/3733#issuecomment-2064940568 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3733) **Quality Gate passed** Issues  [6 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3733=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite=3733=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3733=false=true)  [92.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3733=new_coverage=list)  [17.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3733=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3733) -- 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
Re: [PR] [CALCITE-6340] RelBuilder drops set conventions when aggregating over duplicate projected fields [calcite]
jduo commented on code in PR #3757: URL: https://github.com/apache/calcite/pull/3757#discussion_r1571211748 ## core/src/main/java/org/apache/calcite/tools/RelBuilder.java: ## @@ -2502,9 +2502,10 @@ private RelBuilder pruneAggregateInputFieldsAndDeduplicateAggCalls( newProjects.add(project.getProjects().get(i)); builder.add(project.getRowType().getFieldList().get(i)); } + r = -project.copy(cluster.traitSet(), project.getInput(), newProjects, -builder.build()); +project.copy(project.getTraitSet().apply(targetMapping), project.getInput(), Review Comment: The use of the mapping was to handle the collation and distribution traits. -- 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
Re: [PR] [CALCITE-6340] RelBuilder drops set conventions when aggregating over duplicate projected fields [calcite]
jduo commented on code in PR #3757: URL: https://github.com/apache/calcite/pull/3757#discussion_r1571180866 ## core/src/test/java/org/apache/calcite/test/RelBuilderTest.java: ## @@ -547,6 +547,31 @@ private void checkSimplify(UnaryOperator transform, assertThat(root, matcher); } + /** Test case for + * https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6340;> + * [CALCITE-6340] RelBuilder always creates Project with Convention.NONE during aggregate_.. Review Comment: That description better describes the problem without going into the weeds. -- 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
Re: [PR] [CALCITE-6340] RelBuilder always creates Project with Convention.NONE during aggregate_ [calcite]
jduo commented on code in PR #3757:
URL: https://github.com/apache/calcite/pull/3757#discussion_r1571176366
##
core/src/test/java/org/apache/calcite/test/RelBuilderTest.java:
##
@@ -547,6 +547,31 @@ private void
checkSimplify(UnaryOperator transform,
assertThat(root, matcher);
}
+ /** Test case for
+ * https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6340;>
+ * [CALCITE-6340] RelBuilder always creates Project with Convention.NONE
during aggregate_..
+ */
+ @Test void testPruneProjectInputOfAggregatePreservesTraitSet() {
+final RelBuilder builder = createBuilder(config ->
config.withPruneInputOfAggregate(true));
+
+// This issue only occurs when projecting more columns than there are
fields and putting
+// an aggregate over that projection.
+RelNode root =
+builder.scan("DEPT")
+.adoptConvention(EnumerableConvention.INSTANCE)
+.project(builder.alias(builder.field(0), "a"),
+builder.alias(builder.field(1), "b"),
+builder.alias(builder.field(2), "c"),
+builder.alias(builder.field(1), "d"))
+.aggregate(builder.groupKey(0, 1, 2, 1),
+builder.aggregateCall(SqlStdOperatorTable.SUM,
+builder.field(0)))
+.build();
+
+// Verify that the project under the aggregate kept the
EnumerableConvention.INSTANCE trait.
+
assertTrue(root.getInput(0).getTraitSet().contains(EnumerableConvention.INSTANCE));
Review Comment:
Thanks! Good suggestions @asolimando
--
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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
mihaibudiu commented on PR #3761: URL: https://github.com/apache/calcite/pull/3761#issuecomment-2064785139 This PR seems to implement a few flags which don't even work in BigQuery, but other than that seems fine. -- 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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on PR #3761: URL: https://github.com/apache/calcite/pull/3761#issuecomment-2064728779 @mihaibudiu @tanclary addressed your comments, would appreciate another review on this, 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
Re: [PR] [CALCITE-6340] RelBuilder always creates Project with Convention.NONE during aggregate_ [calcite]
asolimando commented on code in PR #3757:
URL: https://github.com/apache/calcite/pull/3757#discussion_r1571121711
##
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##
@@ -2502,9 +2502,10 @@ private RelBuilder
pruneAggregateInputFieldsAndDeduplicateAggCalls(
newProjects.add(project.getProjects().get(i));
builder.add(project.getRowType().getFieldList().get(i));
}
+
r =
-project.copy(cluster.traitSet(), project.getInput(), newProjects,
-builder.build());
+project.copy(project.getTraitSet().apply(targetMapping),
project.getInput(),
Review Comment:
You don't need any mapping here, the root cause of the issue is that the
previous code is taking the trait set from the cluster instead of the one of
the `Project` it is trying to copy, this is all you need:
```
r = project.copy(project.getTraitSet(), project.getInput(), newProjects,
builder.build());
```
##
core/src/test/java/org/apache/calcite/test/RelBuilderTest.java:
##
@@ -547,6 +547,31 @@ private void
checkSimplify(UnaryOperator transform,
assertThat(root, matcher);
}
+ /** Test case for
+ * https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6340;>
+ * [CALCITE-6340] RelBuilder always creates Project with Convention.NONE
during aggregate_..
+ */
+ @Test void testPruneProjectInputOfAggregatePreservesTraitSet() {
Review Comment:
The test should be moved close to other tests around aggregate, after
`testAggregateEliminatesDuplicateCalls3`, for instance
##
core/src/test/java/org/apache/calcite/test/RelBuilderTest.java:
##
@@ -547,6 +547,31 @@ private void
checkSimplify(UnaryOperator transform,
assertThat(root, matcher);
}
+ /** Test case for
+ * https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6340;>
+ * [CALCITE-6340] RelBuilder always creates Project with Convention.NONE
during aggregate_..
Review Comment:
The Jira link is wrong (you have `projects` instead of `browse` when you
copy it from the project view in Jira):
```suggestion
* https://issues.apache.org/jira/browse/CALCITE/issues/CALCITE-6340;>[CALCITE-6340]
* RelBuilder always creates Project with Convention.NONE during
aggregate_..
```
The ticket title should strive to be as much high-level as possible, the
current title goes too deep into Calcite internals IMO.
Factoring in also the new minimal reproducer below, I would go for something
along the line of "RelBuilder drops set conventions when aggregating over
duplicate projected fields", WDYT?
In case you change the Jira title, please remember to be consistent here,
and in the final commit message too when squashing before merging (only when
the review process is over, though).
##
core/src/test/java/org/apache/calcite/test/RelBuilderTest.java:
##
@@ -547,6 +547,31 @@ private void
checkSimplify(UnaryOperator transform,
assertThat(root, matcher);
}
+ /** Test case for
+ * https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6340;>
+ * [CALCITE-6340] RelBuilder always creates Project with Convention.NONE
during aggregate_..
+ */
+ @Test void testPruneProjectInputOfAggregatePreservesTraitSet() {
+final RelBuilder builder = createBuilder(config ->
config.withPruneInputOfAggregate(true));
+
+// This issue only occurs when projecting more columns than there are
fields and putting
+// an aggregate over that projection.
+RelNode root =
+builder.scan("DEPT")
+.adoptConvention(EnumerableConvention.INSTANCE)
+.project(builder.alias(builder.field(0), "a"),
+builder.alias(builder.field(1), "b"),
+builder.alias(builder.field(2), "c"),
+builder.alias(builder.field(1), "d"))
+.aggregate(builder.groupKey(0, 1, 2, 1),
+builder.aggregateCall(SqlStdOperatorTable.SUM,
+builder.field(0)))
+.build();
+
+// Verify that the project under the aggregate kept the
EnumerableConvention.INSTANCE trait.
+
assertTrue(root.getInput(0).getTraitSet().contains(EnumerableConvention.INSTANCE));
Review Comment:
Test is not really minimal, you just need a single duplicate projected
field, as follows:
```suggestion
final RelNode root = builder.scan("DEPT")
.adoptConvention(EnumerableConvention.INSTANCE)
.project(builder.alias(builder.field(0), "a"),
builder.alias(builder.field(0), "b"))
.aggregate(builder.groupKey(0), builder.aggregateCall(
SqlStdOperatorTable.SUM, builder.field(0))).build();
assertTrue(root.getInput(0).getTraitSet().contains(EnumerableConvention.INSTANCE));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on code in PR #3761:
URL: https://github.com/apache/calcite/pull/3761#discussion_r1571090898
##
core/src/test/resources/sql/cast-with-format.iq:
##
@@ -32,8 +32,15 @@ EXPR$0
2017-05-01 01:23:45
!ok
+# Input that contains shuffled date without time
+select cast('12-2010-05' as timestamp format
+'DD--MM');
+EXPR$0
+2010-05-12 00:00:00
+!ok
+
!if (false) {
-### disabled until Bug.CALCITE_6269_FIXED ###
+### unsupported format model/elements, test disabled ###
Review Comment:
Updated the comments and disable condition to point to the calcite issue,
thanks for suggesting this change!
--
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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on code in PR #3761:
URL: https://github.com/apache/calcite/pull/3761#discussion_r1571090026
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -306,12 +446,22 @@ static Work get() {
* https://issues.apache.org/jira/browse/CALCITE-6252. This may be
* specific to Java 11. */
final DateFormat Format = new SimpleDateFormat(MONTH.javaFmt,
Locale.US);
-final DateFormat sFormat = new SimpleDateFormat(FF1.javaFmt, Locale.ROOT);
-final DateFormat ssFormat = new SimpleDateFormat(FF2.javaFmt, Locale.ROOT);
final DateFormat sssFormat = new SimpleDateFormat(FF3.javaFmt,
Locale.ROOT);
-final DateFormat Format = new SimpleDateFormat(FF4.javaFmt,
Locale.ROOT);
-final DateFormat sFormat = new SimpleDateFormat(FF5.javaFmt,
Locale.ROOT);
-final DateFormat ssFormat = new SimpleDateFormat(FF6.javaFmt,
Locale.ROOT);
final DateFormat yyFormat = new SimpleDateFormat(YY.javaFmt, Locale.ROOT);
+final DateFormat Format = new SimpleDateFormat(.javaFmt,
Locale.ROOT);
+
+/** Util to return the full or abbreviated weekday name from date and
expected TextStyle. */
+public String getDayFromDate(Date date, TextStyle style) {
Review Comment:
Good catch, can be kept private updated it!
--
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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on code in PR #3761:
URL: https://github.com/apache/calcite/pull/3761#discussion_r1571079145
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -217,6 +291,34 @@ public enum FormatElementEnum implements FormatElement {
sb.append(String.format(Locale.ROOT, "%d", (calendar.get(Calendar.MONTH)
/ 3) + 1));
}
},
+ AMPM("", "The time as Meridian Indicator in uppercase") {
+@Override public void format(StringBuilder sb, Date date) {
+ final Calendar calendar = Work.get().calendar;
+ calendar.setTime(date);
+ sb.append(calendar.get(Calendar.AM_PM) == Calendar.AM ? "AM" : "PM");
Review Comment:
Apologies, not an enum but the class stores these calendar elements as
member properties of type int, so the conversion to string is still necessary.
--
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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on code in PR #3761:
URL: https://github.com/apache/calcite/pull/3761#discussion_r1571074953
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -217,6 +291,34 @@ public enum FormatElementEnum implements FormatElement {
sb.append(String.format(Locale.ROOT, "%d", (calendar.get(Calendar.MONTH)
/ 3) + 1));
}
},
+ AMPM("", "The time as Meridian Indicator in uppercase") {
+@Override public void format(StringBuilder sb, Date date) {
+ final Calendar calendar = Work.get().calendar;
+ calendar.setTime(date);
+ sb.append(calendar.get(Calendar.AM_PM) == Calendar.AM ? "AM" : "PM");
Review Comment:
Checked, `Calendar` in an INT enum, so the returned value needs to be
compared and output as a string explicitly, as the enum only contains its int
value.
--
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
Re: [PR] [CALCITE-6269] Fix missing/broken BigQuery date-time format elements [calcite]
Anthrino commented on code in PR #3761: URL: https://github.com/apache/calcite/pull/3761#discussion_r1571070432 ## core/src/test/resources/sql/cast-with-format.iq: ## @@ -979,7 +939,7 @@ EXPR$0 select cast(date'2019-01-07' as varchar FORMAT 'WW'); EXPR$0 -01 +02 Review Comment: Agreed, a small number of the tests return the same answer as recorded here as they produced the same week number irrespective of start of week, the others had to be changed wrt to what Calcite/BQ is returning. Another point to consider with these tests is BQ does not support use of elements like `WW`, `DDD` to parse a datetime object, its only available for formatting output strings. I left these tests as is because we do not have any restrictions in calcite's `FormatModel` for the use of elements in parsing or formatting scenarios. -- 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
Re: [PR] [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule [calcite]
mihaibudiu closed pull request #3763: [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule URL: https://github.com/apache/calcite/pull/3763 -- 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
(calcite) branch main updated: [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule
This is an automated email from the ASF dual-hosted git repository.
mbudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 0551b89033 [CALCITE-5289] Assertion failure in
MultiJoinOptimizeBushyRule
0551b89033 is described below
commit 0551b8903391c1706422a2c1b8b648a6941f39a2
Author: Mihai Budiu
AuthorDate: Wed Apr 17 18:04:30 2024 -0700
[CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule
Signed-off-by: Mihai Budiu
---
.../calcite/rel/rules/MultiJoinOptimizeBushyRule.java | 11 +++
.../test/java/org/apache/calcite/test/RelOptRulesTest.java | 9 +
.../resources/org/apache/calcite/test/RelOptRulesTest.xml | 13 +
3 files changed, 33 insertions(+)
diff --git
a/core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java
index 755df10ab2..e502e8c54c 100644
---
a/core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java
+++
b/core/src/main/java/org/apache/calcite/rel/rules/MultiJoinOptimizeBushyRule.java
@@ -107,6 +107,17 @@ public class MultiJoinOptimizeBushyRule
final RelMetadataQuery mq = call.getMetadataQuery();
final LoptMultiJoin multiJoin = new LoptMultiJoin(multiJoinRel);
+for (int i = 0; i < multiJoin.getNumJoinFactors(); i++) {
+ ImmutableBitSet outerJoinFactors = multiJoin.getOuterJoinFactors(i);
+ if (outerJoinFactors == null) {
+continue;
+ }
+ if (!outerJoinFactors.isEmpty()) {
+// Refuse to apply this rule to a multijoin with outer joins,
+// since this rule cannot handle outer joins.
+return;
+ }
+}
final List vertexes = new ArrayList<>();
int x = 0;
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 7e58a5b775..ab80c1b8fd 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -3383,6 +3383,15 @@ class RelOptRulesTest extends RelOptTestBase {
.check();
}
+ /** Test case for https://issues.apache.org/jira/browse/CALCITE-5289;>
+ * [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule. */
+ @Test void testBushyJoinRule() {
+final String sql = "select emp.ename from emp LEFT JOIN emp AS emp1 on
emp.ename = emp1.ename";
+sql(sql).withPreRule(CoreRules.JOIN_TO_MULTI_JOIN)
+.withRule(CoreRules.MULTI_JOIN_OPTIMIZE_BUSHY)
+.checkUnchanged();
+ }
+
@Test void testConvertMultiJoinRule() {
final String sql = "select e1.ename from emp e1, dept d, emp e2\n"
+ "where e1.deptno = d.deptno and d.deptno = e2.deptno";
diff --git
a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 855a6d5689..237305a629 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -1374,6 +1374,19 @@ LogicalAggregate(group=[{}], EXPR$0=[SUM($0)],
EXPR$1=[COUNT($0)], EXPR$2=[BIT_O
LogicalAggregate(group=[{0}])
LogicalProject(DEPTNO=[$7])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+
+
+
+
+
+
+
+
Re: [PR] [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule [calcite]
mihaibudiu merged PR #3763: URL: https://github.com/apache/calcite/pull/3763 -- 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
Re: [PR] [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule [calcite]
rubenada commented on PR #3763: URL: https://github.com/apache/calcite/pull/3763#issuecomment-2064052951 LGTM -- 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
