You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/15 01:49:24 UTC
[GitHub] [solr] risdenk opened a new pull request #637: SOLR-15904: Create sql module
risdenk opened a new pull request #637:
URL: https://github.com/apache/solr/pull/637
https://issues.apache.org/jira/browse/SOLR-15904
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811286589
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
I understand your point David, and I think if this was a long term thing I would definitely agree. However I don't particularly mind which solution we go with in this case, since it's really a temporary band-aid until we have the solrj sub-modules, which shouldn't be too far off
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r810531401
##########
File path: gradle/validation/spotless.gradle
##########
@@ -53,6 +53,7 @@ configure(project(":solr").subprojects) { prj ->
case ":solr:modules:langid":
case ":solr:modules:ltr":
case ":solr:modules:scripting":
+ case ":solr:modules:sql":
Review comment:
Will be addressed in [SOLR-16031](https://issues.apache.org/jira/browse/SOLR-16031)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r809429024
##########
File path: solr/modules/sql/README.md
##########
@@ -0,0 +1,29 @@
+Apache Solr SQL Module
+===============================
+
+Introduction
+------------
+This module implements the support for SQL in Apache Solr.
+
+Building
+--------
+The SQL module uses the same Gradle build as the core Solr components.
+
+To build the module, you can use
+
+```
+./gradlew :solr:modules:sql:assemble
+```
+
+The resulting module will be placed to the libs directory, for example:
+`solr/modules/hdfs/build/libs/solr-sql-9.0.0-SNAPSHOT.jar`
+
+To execute the module tests:
+
+```
+./gradlew :solr:modules:sql:test
+```
+
+Usage
+-----
+Please refer to the 'Running Solr on HDFS' section of the reference guide: https://solr.apache.org/guide/running-solr-on-hdfs.html
Review comment:
Fixed :D
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806406125
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811236815
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
yeah, definitely don't think they should go in the module, definitely leave them where they are for now. I think this is good to go, and the somewhat-recursive looking dependency (though it's test vs src, so not really recursive) will eventually go away.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1043368147
@HoustonPutman I think you mean https://nightlies.apache.org/solr/draft-guides/solr-reference-guide-antora/solr/10_0/query-guide/sql-query.html potentially? I think it definitely needs a comment about enabling 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811234114
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
@dsmiley had a comment about this on the jira as well regarding moving the solrj tests -> sql module. `JdbcTest` and one of the streamingexpressions jdbc test are what is failing without the `testRuntimeOnly project(':solr:modules:sql')`. I'm not sure it makes sense to move the `solrj` jdbc tests to `solr:modules:sql` since they are really solrj tests. The solrj tests are testing that solrj actually works w/ the SQL module. So I kinda like how the dependencies are defined right now.
Regarding solrj-jdbc module - I would think so unless streaming expressions is somehow still in solrj-core or whatever it will be called.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811228859
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
Whenever the solrj-jdbc solrj-module is split out, this will go in there, correct?
So the dependency tree at that point will look like `solr:solrj:jdbc (test)` -> `solr:modules:sql` -> `solr:solrj`
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806877810
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Created https://issues.apache.org/jira/browse/SOLR-16012 to do this separately.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1040472709
Is there a page for this SQL functionality that we can add the module information to?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1044798261
If you use the PR in github and viewed files it shows exactly what changed. But yea I can try to avoid force pushing. Its clearer to me to make sure the commit is exactly what should be merged.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806401037
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
*Moderate OSS Vulnerability:*
### pkg:maven/com.google.guava/guava@25.1-jre
0 Critical, 0 Severe, 1 Moderate, 0 Unknown vulnerabilities have been found across 1 dependencies
<details>
<summary><b>Components</b></summary><br/>
<ul>
<details>
<summary><b>pkg:maven/com.google.guava/guava@25.1-jre</b></summary>
<ul>
<details>
<summary><b>MODERATE Vulnerabilities (1)</b></summary><br/>
<ul>
> #### [CVE-2020-8908] A temp directory creation vulnerability exists in all versions of Guava, allowin...
> A temp directory creation vulnerability exists in all versions of Guava, allowing an attacker with access to the machine to potentially access data in a temporary directory created by the Guava API com.google.common.io.Files.createTempDir(). By default, on unix-like systems, the created directory is world-readable (readable by an attacker with access to the system). The method in question has been marked @Deprecated in versions 30.0 and later and should not be used. For Android developers, we recommend choosing a temporary directory API provided by Android, such as context.getCacheDir(). For other Java developers, we recommend migrating to the Java 7 API java.nio.file.Files.createTempDirectory() which explicitly configures permissions of 700, or configuring the Java runtime's java.io.tmpdir system property to point to a location whose permissions are appropriately configured.
>
> **CVSS Score:** 3.3
>
> **CVSS Vector:** CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N
</ul>
</details>
</ul>
</details>
</ul>
</details>
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk merged pull request #637: SOLR-15904: Move SQL support into new sql module
Posted by GitBox <gi...@apache.org>.
risdenk merged pull request #637:
URL: https://github.com/apache/solr/pull/637
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1044609919
> The biggest outstanding item is fixing the Implicit Plugins to handle this better. Right now it works, but only if SQL handler is enabled. It breaks otherwise. So need to look into that further.
Sadly I haven't made any progress on this yet. I tracked down how these ImplicitPlugins are referenced and created. It just doesn't look like there is a nice way to ignore the failures.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806401037
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
*Moderate OSS Vulnerability:*
### pkg:maven/com.google.guava/guava@25.1-jre
0 Critical, 0 Severe, 1 Moderate, 0 Unknown vulnerabilities have been found across 1 dependencies
<details>
<summary><b>Components</b></summary><br/>
<ul>
<details>
<summary><b>pkg:maven/com.google.guava/guava@25.1-jre</b></summary>
<ul>
<details>
<summary><b>MODERATE Vulnerabilities (1)</b></summary><br/>
<ul>
> #### [CVE-2020-8908] A temp directory creation vulnerability exists in all versions of Guava, allowin...
> A temp directory creation vulnerability exists in all versions of Guava, allowing an attacker with access to the machine to potentially access data in a temporary directory created by the Guava API com.google.common.io.Files.createTempDir(). By default, on unix-like systems, the created directory is world-readable (readable by an attacker with access to the system). The method in question has been marked @Deprecated in versions 30.0 and later and should not be used. For Android developers, we recommend choosing a temporary directory API provided by Android, such as context.getCacheDir(). For other Java developers, we recommend migrating to the Java 7 API java.nio.file.Files.createTempDirectory() which explicitly configures permissions of 700, or configuring the Java runtime's java.io.tmpdir system property to point to a location whose permissions are appropriately configured.
>
> **CVSS Score:** 3.3
>
> **CVSS Vector:** CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N
</ul>
</details>
</ul>
</details>
</ul>
</details>
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806853966
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Yea I want to make this change outside of this PR. `mapParams` is being used across classes for no good reason. I agree it needs to move somewhere and its a weird method. I'll see where else in `test-framework` makes sense.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806431411
##########
File path: gradle/validation/spotless.gradle
##########
@@ -53,6 +53,7 @@ configure(project(":solr").subprojects) { prj ->
case ":solr:modules:langid":
case ":solr:modules:ltr":
case ":solr:modules:scripting":
+ case ":solr:modules:sql":
Review comment:
LOL I didn't see the context (I needed to expand) and I see you are excluding so no issue then :-). Sorry.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1044796437
@risdenk would you mind not force-pushing your changes? Makes it hard to see what has changed since I last viewed. It's easy to squash and merge in the end anyways 🙂
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1046092200
Updated based on @dsmiley comment in jira. Use startup=lazy and then fix dependencies for tests.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk edited a comment on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk edited a comment on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1043368147
@HoustonPutman I think you mean https://nightlies.apache.org/solr/draft-guides/solr-reference-guide-antora/solr/10_0/query-guide/sql-query.html potentially? I added a note to enable the plugin to use.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806406106
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
@sonatype-lift ignore
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811260743
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
Put differently, if we can make simple moves to reduce intermodule dependencies, even _just_ tests, that's a good thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806406756
##########
File path: gradle/validation/spotless.gradle
##########
@@ -53,6 +53,7 @@ configure(project(":solr").subprojects) { prj ->
case ":solr:modules:langid":
case ":solr:modules:ltr":
case ":solr:modules:scripting":
+ case ":solr:modules:sql":
Review comment:
As I've reminded others, lets please disassociate automatic source code formatting with moving code into a module. By disassociating them, we can point to a commit that can be excluded from history that *only* does formatting and not anything else. These go in `.git-blame-ignore-revs`
##########
File path: solr/core/build.gradle
##########
@@ -153,22 +153,6 @@ dependencies {
implementation 'org.codehaus.woodstox:stax2-api'
implementation 'com.fasterxml.woodstox:woodstox-core'
- // SQL Parser via Calcite
Review comment:
Huzzah!
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Please let's not pollute SolrTestCaseJ4 with this needless method. STCJ4 is out of control and I feel the need to be a policeman. See the existing `params` method. The addition of javabin randomly is weird. I understand you are just trying to move this module and you didn't write this code... so move it somewhere else or consider if it's maybe not useful to randomly alter the response format.
##########
File path: solr/modules/sql/README.md
##########
@@ -0,0 +1,29 @@
+Apache Solr SQL Module
+===============================
+
+Introduction
+------------
+This module implements the support for SQL in Apache Solr.
+
+Building
+--------
+The SQL module uses the same Gradle build as the core Solr components.
+
+To build the module, you can use
+
+```
+./gradlew :solr:modules:sql:assemble
+```
+
+The resulting module will be placed to the libs directory, for example:
+`solr/modules/hdfs/build/libs/solr-sql-9.0.0-SNAPSHOT.jar`
+
+To execute the module tests:
+
+```
+./gradlew :solr:modules:sql:test
+```
+
+Usage
+-----
+Please refer to the 'Running Solr on HDFS' section of the reference guide: https://solr.apache.org/guide/running-solr-on-hdfs.html
Review comment:
Guilty of copy-pasting ;-)
##########
File path: gradle/validation/spotless.gradle
##########
@@ -53,6 +53,7 @@ configure(project(":solr").subprojects) { prj ->
case ":solr:modules:langid":
case ":solr:modules:ltr":
case ":solr:modules:scripting":
+ case ":solr:modules:sql":
Review comment:
LOL I didn't see the context (I needed to expand) and I see you are excluding so no issue then :-). Sorry.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #637:
URL: https://github.com/apache/solr/pull/637#issuecomment-1040472709
Is there a page for this SQL functionality that we can add the module information to?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806406756
##########
File path: gradle/validation/spotless.gradle
##########
@@ -53,6 +53,7 @@ configure(project(":solr").subprojects) { prj ->
case ":solr:modules:langid":
case ":solr:modules:ltr":
case ":solr:modules:scripting":
+ case ":solr:modules:sql":
Review comment:
As I've reminded others, lets please disassociate automatic source code formatting with moving code into a module. By disassociating them, we can point to a commit that can be excluded from history that *only* does formatting and not anything else. These go in `.git-blame-ignore-revs`
##########
File path: solr/core/build.gradle
##########
@@ -153,22 +153,6 @@ dependencies {
implementation 'org.codehaus.woodstox:stax2-api'
implementation 'com.fasterxml.woodstox:woodstox-core'
- // SQL Parser via Calcite
Review comment:
Huzzah!
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Please let's not pollute SolrTestCaseJ4 with this needless method. STCJ4 is out of control and I feel the need to be a policeman. See the existing `params` method. The addition of javabin randomly is weird. I understand you are just trying to move this module and you didn't write this code... so move it somewhere else or consider if it's maybe not useful to randomly alter the response format.
##########
File path: solr/modules/sql/README.md
##########
@@ -0,0 +1,29 @@
+Apache Solr SQL Module
+===============================
+
+Introduction
+------------
+This module implements the support for SQL in Apache Solr.
+
+Building
+--------
+The SQL module uses the same Gradle build as the core Solr components.
+
+To build the module, you can use
+
+```
+./gradlew :solr:modules:sql:assemble
+```
+
+The resulting module will be placed to the libs directory, for example:
+`solr/modules/hdfs/build/libs/solr-sql-9.0.0-SNAPSHOT.jar`
+
+To execute the module tests:
+
+```
+./gradlew :solr:modules:sql:test
+```
+
+Usage
+-----
+Please refer to the 'Running Solr on HDFS' section of the reference guide: https://solr.apache.org/guide/running-solr-on-hdfs.html
Review comment:
Guilty of copy-pasting ;-)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r806406106
##########
File path: solr/modules/sql/build.gradle
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+description = 'SQL Module'
+
+dependencies {
+ implementation project(':solr:core')
+ implementation project(':solr:solrj')
+
+ implementation 'org.slf4j:slf4j-api'
+
+ implementation 'com.google.guava:guava'
+
+ // SQL Parser via Calcite
+ implementation ('org.apache.calcite:calcite-core', {
+ exclude group: 'org.apache.calcite.avatica', module: 'avatica-metrics'
+ exclude group: 'com.google.guava', module: 'guava' // TODO remove this to keep versions in sync
Review comment:
@sonatype-lift ignore
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Yea I want to make this change outside of this PR. `mapParams` is being used across classes for no good reason. I agree it needs to move somewhere and its a weird method. I'll see where else in `test-framework` makes sense.
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -1298,6 +1298,16 @@ public static ModifiableSolrParams params(String... params) {
return Utils.makeMap(params);
}
+ public static SolrParams mapParams(String... vals) {
Review comment:
Created https://issues.apache.org/jira/browse/SOLR-16012 to do this separately.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #637: SOLR-15904: Create sql module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #637:
URL: https://github.com/apache/solr/pull/637#discussion_r811259296
##########
File path: solr/solrj/build.gradle
##########
@@ -62,6 +62,7 @@ dependencies {
testImplementation project(':solr:test-framework')
testImplementation project(':solr:core')
testImplementation project(':solr:solrj')
+ testRuntimeOnly project(':solr:modules:sql')
Review comment:
I disagree but I'm not going to hold this up. If we're adding a dependency between our modules that can be avoided simply by moving a test -- that's a "smell" to me, and the test should move. I think it's natural that a module might have "solrj level tests" to exercise the backend functional by using higher level APIs... that shouldn't mean (IMO) that SolrJ tests need to depend on nearly everything. At least that's the direction this will go if we follow your logic.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org