You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@auron.apache.org by "guixiaowen (via GitHub)" <gi...@apache.org> on 2025/09/22 17:20:07 UTC
[PR] Add an AuronSQLConf class to accumulate description information for each property. #1328 [auron]
guixiaowen opened a new pull request, #1330:
URL: https://github.com/apache/auron/pull/1330
# Which issue does this PR close?
(https://github.com/apache/auron/issues/1328)
# Rationale for this change
Add a class named AuronSQLConf to accumulate description information for each property.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
# How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336664331
Is it possible to make this PR based on https://github.com/apache/auron/pull/1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "lihao712 (via GitHub)" <gi...@apache.org>.
lihao712 commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336687028
> Is it possible to base this PR on #1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
+1
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3328293810
> Thanks for the contribution! CI checks failed due to missing license headers and formatting issues. Could you please add the required license header and run the formatter?
> Thanks for your work. @guixiaowen
>
> Overall, the unified configuration is very helpful. But if Flink integration is on the roadmap, should we make the configuration more general?
@zuston @merrily01 I think so. I also think we should aim for more uniformity. If Flink is incorporated later, we can unify it further. For now, this PR is just about extraction. How does that sound?
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "lihao712 (via GitHub)" <gi...@apache.org>.
lihao712 commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336621180
> > Thanks for the contribution! CI checks failed due to missing license headers and formatting issues. Could you please add the required license header and run the formatter?
>
> > Thanks for your work. @guixiaowen
> > Overall, the unified configuration is very helpful. But if Flink integration is on the roadmap, should we make the configuration more general?
>
> @zuston @merrily01 I think so. I also think we should aim for more uniformity. If Flink is incorporated later, we can unify it further. For now, this PR is just about extraction. How does that sound?
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336822688
> Is it possible to base this PR on #1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
@zuston @wForget @lihao712 @merrily01 +1, I'll close this and submit new changes based this PR on #1340
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add an AuronSQLConf class to accumulate description information for each property. #1328 [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3320325790
@richox ri Could you review this PR when you have time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add an AuronSQLConf class to accumulate description information for each property. #1328 [auron]
Posted by "ShreyeshArangath (via GitHub)" <gi...@apache.org>.
ShreyeshArangath commented on code in PR #1330:
URL: https://github.com/apache/auron/pull/1330#discussion_r2370791816
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/common/AuronSQLConf.scala:
##########
@@ -0,0 +1,153 @@
+package org.apache.spark.sql.auron.common
+
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.internal.SQLConf.buildConf
+
+object AuronSQLConf extends SQLConf {
+
+ val UDF_UDF_JSON_ENABLED =
+ buildConf("spark.auron.udf.UDFJson.enabled")
+ .doc("Enable native implemented get_json_object/json_tuple. May introduce inconsistency in special case (especially with illegal json inputs).")
+ .booleanConf
+ .createWithDefault(true)
+
+ val UDF_BRICK_HOUSE_ENABLED =
+ buildConf("spark.auron.udf.brickhouse.enabled")
+ .doc("Enable some native-implemented brickhouse UDFs.")
+ .booleanConf
+ .createWithDefault(true)
+
+ val DECIMAL_ARITH_OP_ENABLED =
+ buildConf("spark.auron.decimal.arithOp.enabled")
+ .doc("")
+ .booleanConf
+ .createWithDefault(false)
+
+ val ENABLED_SCAN =
+ buildConf("spark.auron.enable.scan")
+ .doc("Enable/disable auron engine.")
Review Comment:
The doc here does not seem complete
```suggestion
.doc("Enable/disable auron engine for scans.")
```
?
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/common/AuronSQLConf.scala:
##########
@@ -0,0 +1,153 @@
+package org.apache.spark.sql.auron.common
+
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.internal.SQLConf.buildConf
+
+object AuronSQLConf extends SQLConf {
+
+ val UDF_UDF_JSON_ENABLED =
+ buildConf("spark.auron.udf.UDFJson.enabled")
+ .doc("Enable native implemented get_json_object/json_tuple. May introduce inconsistency in special case (especially with illegal json inputs).")
+ .booleanConf
+ .createWithDefault(true)
+
+ val UDF_BRICK_HOUSE_ENABLED =
+ buildConf("spark.auron.udf.brickhouse.enabled")
+ .doc("Enable some native-implemented brickhouse UDFs.")
+ .booleanConf
+ .createWithDefault(true)
+
+ val DECIMAL_ARITH_OP_ENABLED =
+ buildConf("spark.auron.decimal.arithOp.enabled")
+ .doc("")
Review Comment:
Should we add doc here? This particularly stands out as its disabled by default
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "lihao712 (via GitHub)" <gi...@apache.org>.
lihao712 closed pull request #1330: [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property.
URL: https://github.com/apache/auron/pull/1330
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen closed pull request #1330: [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property.
URL: https://github.com/apache/auron/pull/1330
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm commented on code in PR #1330:
URL: https://github.com/apache/auron/pull/1330#discussion_r2371740704
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/AuronConverters.scala:
##########
@@ -1097,6 +1095,16 @@ object AuronConverters extends Logging {
projections.map(kv => Alias(kv._1, kv._2.name)(kv._2.exprId)).toList))
}
+ def getBooleanConf(key: String): Boolean = {
Review Comment:
What are the concerns about removing the default value here?
how about like this
```
def getBooleanConf(conf: SQLConf): Boolean = {
getBooleanConf(conf.key, conf.defaultValueString)
}
```
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336671435
> Is it possible to base this PR on #1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
+1, I also prefer this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #1330:
URL: https://github.com/apache/auron/pull/1330#discussion_r2380701858
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/common/AuronSQLConf.scala:
##########
@@ -0,0 +1,168 @@
+/*
+ * 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.
+ */
+package org.apache.spark.sql.auron.common
+
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.internal.SQLConf.buildConf
+
+object AuronSQLConf extends SQLConf {
+
+ val UDF_UDF_JSON_ENABLED =
+ buildConf("spark.auron.udf.UDFJson.enabled")
+ .doc("Enable native implemented get_json_object/json_tuple. May introduce inconsistency in special case (especially with illegal json inputs).")
+ .booleanConf
+ .createWithDefault(true)
+
+ val UDF_BRICK_HOUSE_ENABLED =
+ buildConf("spark.auron.udf.brickhouse.enabled")
+ .doc("Enable some native-implemented brickhouse UDFs.")
+ .booleanConf
+ .createWithDefault(true)
+
+ val DECIMAL_ARITH_OP_ENABLED =
+ buildConf("spark.auron.decimal.arithOp.enabled")
+ .booleanConf
+ .createWithDefault(false)
+
+ val ENABLED_SCAN =
+ buildConf("spark.auron.enable.scan")
Review Comment:
Could you fix the naming style of these configuration keys? For example, change `spark.auron.enable.scan` to `spark.auron.scan.enabled`
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "lihao712 (via GitHub)" <gi...@apache.org>.
lihao712 commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336661779
> > Thanks for the contribution! CI checks failed due to missing license headers and formatting issues. Could you please add the required license header and run the formatter?
>
> > Thanks for your work. @guixiaowen
> > Overall, the unified configuration is very helpful. But if Flink integration is on the roadmap, should we make the configuration more general?
>
> @zuston @merrily01 I think so. I also think we should aim for more uniformity. If Flink is incorporated later, we can unify it further. For now, this PR is just about extraction. How does that sound?
@guixiaowen Sorry, I accidentally closed the issue earlier. Considering the discussion above, wouldn't 'AuronSparkSQLConf' be a better name
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336689331
> Is it possible to base this PR on #1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
@zuston +1, Following the introduction of the new Config design, an AuronSparkConfiguration can be introduced for Spark based on AuronConfiguration.
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336554706
> For now, this PR is just about extraction. How does that sound?
Thanks, that makes sense to me.
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "lihao712 (via GitHub)" <gi...@apache.org>.
lihao712 commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336623709
> > Thanks for the contribution! CI checks failed due to missing license headers and formatting issues. Could you please add the required license header and run the formatter?
>
> > Thanks for your work. @guixiaowen
> > Overall, the unified configuration is very helpful. But if Flink integration is on the roadmap, should we make the configuration more general?
>
> @zuston @merrily01 I think so. I also think we should aim for more uniformity. If Flink is incorporated later, we can unify it further. For now, this PR is just about extraction. How does that sound?
I think ‘AuronSparkSQLConf ’ is less ambiguous
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3336684175
> Is it possible to base this PR on #1340 ? @wForget @lihao712 @guixiaowen @Tartarus0zm
I have no preference on this. I just want to bring these key name fixes into v6.0.0 to reduce user behavior changes in subsequent versions.
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1330] Add an AuronSQLConf class to accumulate description information for each property. [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #1330:
URL: https://github.com/apache/auron/pull/1330#issuecomment-3322584630
> Mostly LGTM, thanks for doing this.
>
> We probably also need to update the PR title to include the issue #
OK.
--
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-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org