[GitHub] [calcite] julianhyde commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
julianhyde commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1035407621 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: I've made some further remarks in the jira case, and +1 when those are fixed. -- 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 #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
julianhyde commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1035403678 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: I think proj4j are going to fix the licensing issue on their end. I think a CI job would be overkill given that this fix will be short-lived. I suggest adding a 'Add Proj4j as a dependency' Jira case, to be fixed in 1.34, and revisit then. -- 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-5259] Add getParameterRowType method to Planner interface
This is an automated email from the ASF dual-hosted git repository.
jbalint 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 531dc3d3fa [CALCITE-5259] Add getParameterRowType method to Planner
interface
531dc3d3fa is described below
commit 531dc3d3fab1f882dd609f419683177df6c57c77
Author: dssysolyatin
AuthorDate: Thu Sep 1 12:25:42 2022 +0300
[CALCITE-5259] Add getParameterRowType method to Planner interface
---
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java | 9 +
core/src/main/java/org/apache/calcite/tools/Planner.java | 8
2 files changed, 17 insertions(+)
diff --git a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
index a16c5a8a27..a845129135 100644
--- a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
@@ -237,6 +237,15 @@ public class PlannerImpl implements Planner, ViewExpander {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
+
+return requireNonNull(validator, "validator")
+.getParameterRowType(requireNonNull(validatedSqlNode,
"validatedSqlNode"));
+ }
+
@SuppressWarnings("deprecation")
@Override public final RelNode convert(SqlNode sql) {
return rel(sql).rel;
diff --git a/core/src/main/java/org/apache/calcite/tools/Planner.java
b/core/src/main/java/org/apache/calcite/tools/Planner.java
index 11de3da0d8..afaf160c74 100644
--- a/core/src/main/java/org/apache/calcite/tools/Planner.java
+++ b/core/src/main/java/org/apache/calcite/tools/Planner.java
@@ -78,6 +78,14 @@ public interface Planner extends AutoCloseable {
*/
Pair validateAndGetType(SqlNode sqlNode) throws
ValidationException;
+ /**
+ * Returns a record type that contains the name and type of each parameter.
+ * Returns a record type with no fields if there are no parameters.
+ *
+ * @return Record type
+ */
+ RelDataType getParameterRowType();
+
/**
* Converts a SQL parse tree into a tree of relational expressions.
*
[GitHub] [calcite] jbalint merged pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint merged PR #2890: URL: https://github.com/apache/calcite/pull/2890 -- 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] jbalint commented on a diff in pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1035051360
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
yes - you're right. the javadoc is wrong... :(
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` to the CI be sufficient? Should it be a dedicated github actions job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? Should it be a dedicated github actions job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? Should it be a dedicated job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? -- 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 #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
dssysolyatin commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1034930593
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
@jbalint
`ensure` works differently. `ensure` checks if planner can move to next
state. if ensure is used and user calls getParameterRowType from
STATE_5_CONVERTED state, the `getParameterRowType` will throw exception
--
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] jbalint commented on a diff in pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1034916228
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
```suggestion
ensure(State.STATE_4_VALIDATED);```
--
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] 01/02: [CALCITE-5217] Add support for INTERVAL qualifier for Firebolt
This is an automated email from the ASF dual-hosted git repository.
jbalint pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit bae84c7e17e038169fefb7119bfd43af6707f8da
Author: Aymeric
AuthorDate: Sat Jul 23 09:50:04 2022 +0100
[CALCITE-5217] Add support for INTERVAL qualifier for Firebolt
---
.../apache/calcite/sql/dialect/FireboltSqlDialect.java| 15 +++
.../apache/calcite/rel/rel2sql/RelToSqlConverterTest.java | 10 ++
2 files changed, 25 insertions(+)
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
index 25a4859fd8..d76e902a27 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
@@ -23,6 +23,7 @@ import org.apache.calcite.sql.SqlAlienSystemTypeNameSpec;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlDataTypeSpec;
import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.SqlIntervalLiteral;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
@@ -200,4 +201,18 @@ public class FireboltSqlDialect extends SqlDialect {
}
}
}
+
+ @Override public void unparseSqlIntervalLiteral(SqlWriter writer,
+ SqlIntervalLiteral literal, int leftPrec, int rightPrec) {
+SqlIntervalLiteral.IntervalValue interval =
+literal.getValueAs(SqlIntervalLiteral.IntervalValue.class);
+writer.keyword("INTERVAL");
+writer.print("'");
+if (interval.getSign() == -1) {
+ writer.print("-");
+}
+writer.literal(interval.getIntervalLiteral());
+writer.print(interval.getIntervalQualifier().toString());
+writer.print("'");
+ }
}
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 5ef9e02126..05ef6e03c4 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -4004,6 +4004,16 @@ class RelToSqlConverterTest {
sql(sql2).withBigQuery().throws_("Only INT64 is supported as the interval
value for BigQuery.");
}
+ @Test void testUnparseSqlIntervalQualifierFirebolt() {
+final String sql = "select * from \"employee\" where \"hire_date\" + "
++ "INTERVAL '10' HOUR > TIMESTAMP '2005-10-17 00:00:00' ";
+
+final String expect = "SELECT *\n"
++ "FROM \"foodmart\".\"employee\"\n"
++ "WHERE (\"hire_date\" + INTERVAL '10 HOUR')"
++ " > TIMESTAMP '2005-10-17 00:00:00'";
+sql(sql).withFirebolt().ok(expect);
+ }
@Test void testFloorMysqlWeek() {
String query = "SELECT floor(\"hire_date\" TO WEEK) FROM \"employee\"";
String expected = "SELECT STR_TO_DATE(DATE_FORMAT(`hire_date` , '%x%v-1'),
'%x%v-%w')\n"
[calcite] 02/02: [CALCITE-5217] Improve support for INTERVAL qualifier for Firebolt
This is an automated email from the ASF dual-hosted git repository.
jbalint pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 35c879044c3a1178760009fae0dd583e79856371
Author: Aymeric
AuthorDate: Wed Jul 27 17:04:23 2022 +0100
[CALCITE-5217] Improve support for INTERVAL qualifier for Firebolt
---
.../calcite/sql/dialect/FireboltSqlDialect.java| 44 --
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 22 ---
2 files changed, 58 insertions(+), 8 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
index d76e902a27..0f005bd307 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
@@ -16,14 +16,17 @@
*/
package org.apache.calcite.sql.dialect;
+import org.apache.calcite.avatica.util.TimeUnit;
import org.apache.calcite.avatica.util.TimeUnitRange;
import org.apache.calcite.config.NullCollation;
import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeSystem;
import org.apache.calcite.sql.SqlAlienSystemTypeNameSpec;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlDataTypeSpec;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.SqlIntervalLiteral;
+import org.apache.calcite.sql.SqlIntervalQualifier;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
@@ -202,17 +205,52 @@ public class FireboltSqlDialect extends SqlDialect {
}
}
+ /** Firebolt interval syntax: sign INTERVAL int64 time_unit. */
@Override public void unparseSqlIntervalLiteral(SqlWriter writer,
SqlIntervalLiteral literal, int leftPrec, int rightPrec) {
SqlIntervalLiteral.IntervalValue interval =
literal.getValueAs(SqlIntervalLiteral.IntervalValue.class);
writer.keyword("INTERVAL");
writer.print("'");
-if (interval.getSign() == -1) {
- writer.print("-");
+try {
+ Long.parseLong(interval.getIntervalLiteral());
+} catch (NumberFormatException e) {
+ throw new RuntimeException("Only INT64 is supported as the interval
value for Firebolt.");
}
writer.literal(interval.getIntervalLiteral());
-writer.print(interval.getIntervalQualifier().toString());
+unparseSqlIntervalQualifier(writer, interval.getIntervalQualifier(),
+RelDataTypeSystem.DEFAULT);
writer.print("'");
}
+
+ @Override public void unparseSqlIntervalQualifier(
+ SqlWriter writer, SqlIntervalQualifier qualifier, RelDataTypeSystem
typeSystem) {
+final String start = validate(qualifier.timeUnitRange.startUnit).name();
+if (qualifier.timeUnitRange.endUnit == null) {
+ writer.keyword(start);
+} else {
+ throw new RuntimeException("Range time unit is not supported for
Firebolt.");
+}
+ }
+
+ private static TimeUnit validate(TimeUnit timeUnit) {
+switch (timeUnit) {
+case MICROSECOND:
+case MILLISECOND:
+case SECOND:
+case MINUTE:
+case HOUR:
+case DAY:
+case WEEK:
+case MONTH:
+case YEAR:
+case DECADE:
+case CENTURY:
+case MILLENNIUM:
+ return timeUnit;
+default:
+ throw new RuntimeException("Time unit " + timeUnit + " is not supported
for Firebolt.");
+}
+ }
+
}
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 05ef6e03c4..0cbda44c6b 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -4005,15 +4005,27 @@ class RelToSqlConverterTest {
}
@Test void testUnparseSqlIntervalQualifierFirebolt() {
-final String sql = "select * from \"employee\" where \"hire_date\" + "
-+ "INTERVAL '10' HOUR > TIMESTAMP '2005-10-17 00:00:00' ";
+final String sql0 = "select * from \"employee\" where \"hire_date\" - "
++ "INTERVAL '19800' SECOND(5) > TIMESTAMP '2005-10-17 00:00:00' ";
+final String expect0 = "SELECT *\n"
++ "FROM \"foodmart\".\"employee\"\n"
++ "WHERE (\"hire_date\" - INTERVAL '19800 SECOND ')"
++ " > TIMESTAMP '2005-10-17 00:00:00'";
+sql(sql0).withFirebolt().ok(expect0);
-final String expect = "SELECT *\n"
+final String sql1 = "select * from \"employee\" where \"hire_date\" + "
++ "INTERVAL '10' HOUR > TIMESTAMP '2005-10-17 00:00:00' ";
+final String expect1 = "SELECT *\n"
+ "FROM \"foodmart\".\"employee\"\n"
-+ "WHERE (\"hire_date\" + INTERVAL '10 HOUR')"
++ "WHERE (\"hire_date\" + INTERVAL '10 HOUR ')"
+ " > TIMESTAMP '2005-10-17 00:00:
[calcite] branch main updated (52922f9a67 -> 35c879044c)
This is an automated email from the ASF dual-hosted git repository. jbalint pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git from 52922f9a67 Upgrade jackson to 2.14.1 new bae84c7e17 [CALCITE-5217] Add support for INTERVAL qualifier for Firebolt new 35c879044c [CALCITE-5217] Improve support for INTERVAL qualifier for Firebolt The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../calcite/sql/dialect/FireboltSqlDialect.java| 53 ++ .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 22 + 2 files changed, 75 insertions(+)
[GitHub] [calcite] jbalint merged pull request #2860: [CALCITE-5217] Add support for INTERVAL qualifier for Firebolt
jbalint merged PR #2860: URL: https://github.com/apache/calcite/pull/2860 -- 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 pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
dssysolyatin commented on PR #2819: URL: https://github.com/apache/calcite/pull/2819#issuecomment-1330769024 @julianhyde if you have time, please review this PR again. This PR is actively used in one of our project, I have to fix conflicts quite often. 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I created a simple project, and it confirms that everything
behave as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I created a simple project and it confirms that everything behave
as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I create a simple project and it confirms that everything behave
as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034737271
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
I replaced `testImplementation` by `testRuntimeOnly`, however, builds fail
without `compileOnly`. I also tried to improve grouping but failed at
identifying a clear pattern (alphabetical order, etc.). I simply moved the
spatial related dependencies at the end of each group.
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034723743
##
core/src/main/java/org/apache/calcite/runtime/ProjectionTransformer.java:
##
@@ -40,6 +40,14 @@
/**
* Transforms the projection of a geometry.
+ *
+ * This class uses Proj4J to transform the projection of a geometry
+ * and should not be used beyond the scope of the Spatial Type Extensions.
+ * Proj4J is released under the Apache License 2.0, however, it also uses the
EPSG dataset,
+ * which has restricting https://epsg.org/terms-of-use.html";>terms of
use.
+ * As a result, Proj4J is not suitable for inclusion in Apache Calcite
+ * and this class will throw {@code ClassNotFoundException}s
+ * if Proj4J is not added to the classpath by the user.
Review Comment:
This is a good idea, I added notices in `reference.md` and `spatial.md`.
--
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 #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
zabetak commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034587984
##
site/_docs/howto.md:
##
@@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above.
that the `META-INF` directory contains `LICENSE`,
`NOTICE`
* Check PGP, per [this](https://httpd.apache.org/dev/verification.html)
+* Check that Proj4J is not among the api and implementation dependencies,
Review Comment:
We could enforce this with a new CI job as well. Not necessary to do it now
but maybe worth considering as a follow-up.
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
`testImplementation` means that we need it in the compile and runtime
classpath for tests; is this true? If we can use `testRuntimeOnly` I think it
would be better.
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
Moreover, consider moving it closer to other dependencies in the same group.
--
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 #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1330293444 @julianhyde Very thanks for the reminder đź‘Ť , I did plan to do that while merging. -- 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 pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
julianhyde commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1330281838 @libenchao when you merge, please change the commit message to something that doesn’t start with “fix” -- 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 a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
rubenada commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034434709
##
core/src/main/java/org/apache/calcite/runtime/ProjectionTransformer.java:
##
@@ -40,6 +40,14 @@
/**
* Transforms the projection of a geometry.
+ *
+ * This class uses Proj4J to transform the projection of a geometry
+ * and should not be used beyond the scope of the Spatial Type Extensions.
+ * Proj4J is released under the Apache License 2.0, however, it also uses the
EPSG dataset,
+ * which has restricting https://epsg.org/terms-of-use.html";>terms of
use.
+ * As a result, Proj4J is not suitable for inclusion in Apache Calcite
+ * and this class will throw {@code ClassNotFoundException}s
+ * if Proj4J is not added to the classpath by the user.
Review Comment:
Would we need to add a similar warning in the web documentation
(`reference.md`) in the spatial section?
--
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
