Re: [PR] [CALCITE-6339] Replace hashicorp/go-uuid with google/uuid [calcite-avatica-go]
F21 merged PR #75: URL: https://github.com/apache/calcite-avatica-go/pull/75 -- 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-avatica-go) branch main updated (a4daffd -> 6959af3)
This is an automated email from the ASF dual-hosted git repository. francischuang pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite-avatica-go.git from a4daffd Bump google.golang.org/protobuf from 1.31.0 to 1.33.0 add 6959af3 [CALCITE-6339] Replace hashicorp/go-uuid with google/uuid No new revisions were added by this update. Summary of changes: driver.go | 6 +++--- go.mod| 3 ++- go.sum| 2 ++ 3 files changed, 7 insertions(+), 4 deletions(-)
Re: [PR] [CALCITE-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1537510286
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6332,6 +6346,66 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6325;>[CALCITE-6325]
+ * Add LOG function (enabled in MYSQL, Spark library). */
Review Comment:
Sorry, I was sick during this period and was delayed for some 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
Re: [PR] [CALCITE-6325] Add LOG function (enabled in Mysql library) [calcite]
sonarcloud[bot] commented on PR #3732: URL: https://github.com/apache/calcite/pull/3732#issuecomment-2017916618 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3732) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3732=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite=3732=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3732=false=true)  [98.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3732=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3732=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3732) -- 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-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE f… [calcite]
barrkel commented on code in PR #3336:
URL: https://github.com/apache/calcite/pull/3336#discussion_r1537365076
##
core/src/main/java/org/apache/calcite/sql/SqlOperator.java:
##
@@ -537,6 +538,11 @@ public RelDataType inferReturnType(
+ opBinding.collectOperandTypes());
}
+ // MEASURE wrapper should be removed, e.g. MEASURE should just
be DOUBLE
+ if (isMeasure(returnType) && returnType.getMeasureElementType() != null)
{
+returnType =
Objects.requireNonNull(returnType.getMeasureElementType());
Review Comment:
I am leaving this comment to highlight relevant code to ask @tanclary about
it.
This strips off the `MEASURE<>`-ness from `SELECT`ed columns when the `AS`
operator is used - aliases are modeled in the AST as an operator.
This means that if
```
SELECT m FROM table
```
has `m` of type `MEASURE`, then
```
SELECT m AS m FROM table
```
will have `m` of type `INTEGER`. This then causes breakage when the AST
validator has a different opinion about the type of the row than the conversion
into rel nodes.
I want to understand more about the requirement to strip off `MEASURE`. I
think it shouldn't be done for `AS`, but if I can understand why it does need
to be done, then there might be more cases where it shouldn't be 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
(calcite) branch main updated: [CALCITE-6338] RelMdCollation#project can return an incomplete list of collations in the presence of aliasing
This is an automated email from the ASF dual-hosted git repository.
rubenql 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 d5b6b5c01f [CALCITE-6338] RelMdCollation#project can return an
incomplete list of collations in the presence of aliasing
d5b6b5c01f is described below
commit d5b6b5c01ff17cac100a591a4cc4c9e83d4e1ace
Author: Ruben Quesada Lopez
AuthorDate: Thu Mar 21 16:43:37 2024 +
[CALCITE-6338] RelMdCollation#project can return an incomplete list of
collations in the presence of aliasing
---
.../calcite/rel/metadata/RelMdCollation.java | 20 ---
.../org/apache/calcite/test/RelMetadataTest.java | 40 ++
core/src/test/resources/sql/sub-query.iq | 5 ++-
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java
index 7c107d39d6..9c728384f4 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java
@@ -316,22 +316,32 @@ public class RelMdCollation
targetsWithMonotonicity.put(project.i,
call.getOperator().getMonotonicity(binding));
}
}
-final List fieldCollations = new ArrayList<>();
+List> fieldCollationsList = new ArrayList<>();
loop:
for (RelCollation ic : inputCollations) {
if (ic.getFieldCollations().isEmpty()) {
continue;
}
- fieldCollations.clear();
+ fieldCollationsList.clear();
+ fieldCollationsList.add(new ArrayList<>());
for (RelFieldCollation ifc : ic.getFieldCollations()) {
final Collection integers = targets.get(ifc.getFieldIndex());
if (integers.isEmpty()) {
continue loop; // cannot do this collation
}
-fieldCollations.add(ifc.withFieldIndex(integers.iterator().next()));
+fieldCollationsList = fieldCollationsList.stream()
+.flatMap(fieldCollations -> integers.stream()
+.map(integer -> {
+ List newFieldCollations = new
ArrayList<>(fieldCollations);
+ newFieldCollations.add(ifc.withFieldIndex(integer));
+ return newFieldCollations;
+})).collect(Collectors.toList());
+ }
+ assert !fieldCollationsList.isEmpty();
+ for (List fieldCollations : fieldCollationsList) {
+assert !fieldCollations.isEmpty();
+collations.add(RelCollations.of(fieldCollations));
}
- assert !fieldCollations.isEmpty();
- collations.add(RelCollations.of(fieldCollations));
}
final List fieldCollationsForRexCalls =
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index db1f65c242..d9733486bd 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -1948,6 +1948,46 @@ public class RelMetadataTest {
assertEquals("[[0]]", collations.toString());
}
+ /**
+ * Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6338;>[CALCITE-6338]
+ * RelMdCollation#project can return an incomplete list of collations
+ * in the presence of aliasing.
+ */
+ @Test void testCollationProjectAliasing() {
+final RelBuilder builder = RelBuilderTest.createBuilder();
+final RelNode relNode1 = builder
+.scan("EMP")
+.sort(2, 3)
+.project(builder.field(0), builder.field(2), builder.field(2),
builder.field(3))
+.build();
+checkCollationProjectAliasing(relNode1, "[[1, 3], [2, 3]]");
+final RelNode relNode2 = builder
+.scan("EMP")
+.sort(0, 1)
+.project(builder.field(0), builder.field(0), builder.field(1),
builder.field(1))
+.build();
+checkCollationProjectAliasing(relNode2, "[[0, 2], [0, 3], [1, 2], [1,
3]]");
+final RelNode relNode3 = builder
+.scan("EMP")
+.sort(0, 1, 2)
+.project(
+builder.field(0), builder.field(0),
+builder.field(1), builder.field(1), builder.field(1),
+builder.field(2))
+.build();
+checkCollationProjectAliasing(relNode3,
+"[[0, 2, 5], [0, 3, 5], [0, 4, 5], [1, 2, 5], [1, 3, 5], [1, 4, 5]]");
+ }
+
+ private void checkCollationProjectAliasing(RelNode relNode, String
expectedCollation) {
+final RelMetadataQuery mq = relNode.getCluster().getMetadataQuery();
+assertThat(relNode, instanceOf(Project.class));
+final ImmutableList collations = mq.collations(relNode);
+assertThat(collations, notNullValue());
+assertEquals(expectedCollation, collations.toString());
+ }
+
/** Unit test for
* {@link
Re: [PR] [CALCITE-6300] Function MAP_VALUES/MAP_KEYS gives exception when mapVauleType and mapKeyType not equals map Biggest mapKeytype or mapValueType [calcite]
caicancai commented on PR #3721: URL: https://github.com/apache/calcite/pull/3721#issuecomment-2017898601 @mihaibudiu If you have time, can you review it for me? Thank you. -- 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-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE f… [calcite]
barrkel commented on code in PR #3336:
URL: https://github.com/apache/calcite/pull/3336#discussion_r1537365076
##
core/src/main/java/org/apache/calcite/sql/SqlOperator.java:
##
@@ -537,6 +538,11 @@ public RelDataType inferReturnType(
+ opBinding.collectOperandTypes());
}
+ // MEASURE wrapper should be removed, e.g. MEASURE should just
be DOUBLE
+ if (isMeasure(returnType) && returnType.getMeasureElementType() != null)
{
+returnType =
Objects.requireNonNull(returnType.getMeasureElementType());
Review Comment:
I am leaving this comment to highlight relevant code to ask @tanclary about
it.
This strips off the `MEASURE<>`-ness from `SELECT`ed columns when the `AS`
operator is used - aliases are modeled in the AST as an operator.
This means that if
```
SELECT m FROM table
```
has `m` of type `MEASURE`, then
```
SELECT m AS m FROM table
```
will have `m` of type `INTEGER`. This then causes breakage when the AST
validator has a different opinion about the type of the row than the conversion
into rel nodes.
I want to understand more about the requirement to strip of `MEASURE`. I
think it shouldn't be done for `AS`, but if I can understand why it does need
to be done, then there might be more cases where it shouldn't be 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
Re: [PR] [CALCITE-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE f… [calcite]
barrkel commented on code in PR #3336:
URL: https://github.com/apache/calcite/pull/3336#discussion_r1537378040
##
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##
@@ -896,6 +900,12 @@ public static boolean canCastFrom(
if (toType.equals(fromType)) {
return true;
}
+// If fromType is a measure, you should compare the inner type otherwise
it will
+// always return TRUE because the SqlTypeFamily of MEASURE is ANY
+if (isMeasure(fromType)) {
Review Comment:
@tanclary If I understand this code correctly, this is testing for implicit
(I think) conversions presumably as part of type checking on behalf of the
caller. But it's not wholly clear to me that measures ought to be implicitly
convertible to anything without applying `AGGREGATE`. Is it meaningful to
compute anything with measures in themselves, outside an aggregation context?
At the very least, there's an assumption of naked measures and implicit
aggregation, right?
--
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-6338] RelMdCollation#project can return an incomplete list of collations in the presence of aliasing [calcite]
rubenada merged PR #3739: URL: https://github.com/apache/calcite/pull/3739 -- 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-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE f… [calcite]
tanclary commented on code in PR #3336:
URL: https://github.com/apache/calcite/pull/3336#discussion_r1537841464
##
core/src/main/java/org/apache/calcite/sql/SqlOperator.java:
##
@@ -537,6 +538,11 @@ public RelDataType inferReturnType(
+ opBinding.collectOperandTypes());
}
+ // MEASURE wrapper should be removed, e.g. MEASURE should just
be DOUBLE
+ if (isMeasure(returnType) && returnType.getMeasureElementType() != null)
{
+returnType =
Objects.requireNonNull(returnType.getMeasureElementType());
Review Comment:
This is originally what I thought too. See the above comment chain between
me, Julian, and TJ.
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
tanclary commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1537851006
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6332,6 +6338,53 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6325;>[CALCITE-6325]
+ * Add LOG function (enabled in MYSQL library). */
Review Comment:
Please fix the capitalization
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
tanclary commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1537849935
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -4127,14 +4130,14 @@ private static class LogicalNotImplementor extends
AbstractRexCallImplementor {
* Handles all logarithm functions using log rules to determine the
* appropriate base (i.e. base e for LN).
*/
- private static class LogImplementor extends AbstractRexCallImplementor {
-LogImplementor() {
- super("log", NullPolicy.STRICT, true);
+ private static class LogImplementor extends MethodImplementor {
+LogImplementor(Method method) {
+ super(method, NullPolicy.STRICT, true);
Review Comment:
That sounds fine but why are you extending MethodImplementor and not
AbstractRexCallImplementor
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1538391118
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -4127,14 +4130,14 @@ private static class LogicalNotImplementor extends
AbstractRexCallImplementor {
* Handles all logarithm functions using log rules to determine the
* appropriate base (i.e. base e for LN).
*/
- private static class LogImplementor extends AbstractRexCallImplementor {
-LogImplementor() {
- super("log", NullPolicy.STRICT, true);
+ private static class LogImplementor extends MethodImplementor {
+LogImplementor(Method method) {
+ super(method, NullPolicy.STRICT, true);
Review Comment:
@tanclary Hello，I have tried it before. Using AbstractRexCallImplementor
means that I need to pass a parameter to determine whether the log function
belongs to mysql or bigquery. I can completely locate it through method and
method name. This does not require me to make a judgment (because method name
can be directly mapped to Sqlfunctions method), I think MethodImplementor is
more suitable in terms of code size and simplicity, thank you
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1538420221
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6332,6 +6338,53 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6325;>[CALCITE-6325]
+ * Add LOG function (enabled in MYSQL library). */
Review Comment:
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
Re: [PR] [CALCITE-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1538388201
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6332,6 +6338,53 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6325;>[CALCITE-6325]
+ * Add LOG function (enabled in MYSQL library). */
Review Comment:
Mysql?
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1538388201
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6332,6 +6338,53 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6325;>[CALCITE-6325]
+ * Add LOG function (enabled in MYSQL library). */
Review Comment:
Mysql?
--
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-6325] Add LOG function (enabled in Mysql library) [calcite]
caicancai commented on code in PR #3732:
URL: https://github.com/apache/calcite/pull/3732#discussion_r1538391118
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -4127,14 +4130,14 @@ private static class LogicalNotImplementor extends
AbstractRexCallImplementor {
* Handles all logarithm functions using log rules to determine the
* appropriate base (i.e. base e for LN).
*/
- private static class LogImplementor extends AbstractRexCallImplementor {
-LogImplementor() {
- super("log", NullPolicy.STRICT, true);
+ private static class LogImplementor extends MethodImplementor {
+LogImplementor(Method method) {
+ super(method, NullPolicy.STRICT, true);
Review Comment:
@tanclary Hello，I have tried it before. Using AbstractRexCallImplementor
means that I need to pass a parameter to determine whether the log function
belongs to mysql or bigquery. I can completely locate it through method and
method name. This does not require me to make a judgment (because method name
can be directly mapped to Sqlfunctions method).
I think MethodImplementor is more suitable in terms of code size and
simplicity, thank you
--
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
