You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2026/09/14 18:55:14 UTC
[M] Change in asterixdb[master]: [ASTERIXDB-3843][COMP]:Add plain integer support for the CLUSTER BY d...
From Shahrzad Shirazi <sh...@ucr.edu>:
Shahrzad Shirazi has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21790?usp=email )
Change subject: [ASTERIXDB-3843][COMP]:Add plain integer support for the CLUSTER BY dimension
......................................................................
[ASTERIXDB-3843][COMP]:Add plain integer support for the CLUSTER BY dimension
- user model changes: no
- storage format changes: no
- interface changes: no
Ext-ref: MB-73946
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
Change-Id: Ifa0ada68e905fb97fd4256aa98f9cd75af830414
---
M asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/ClusterByQueries.xml
C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/kmeans-syntax/kmeans-syntax.4.query.sqlpp
R asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-wrong-type/test.1.query.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/results/cluster-by/kmeans-syntax/kmeans-syntax.4.adm
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppClusterByVisitor.java
5 files changed, 42 insertions(+), 27 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/90/21790/1
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/ClusterByQueries.xml b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/ClusterByQueries.xml
index bb239df..437e6e8 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/ClusterByQueries.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/ClusterByQueries.xml
@@ -133,9 +133,9 @@
</compilation-unit>
</test-case>
<test-case FilePath="cluster-by/negative">
- <compilation-unit name="dimension-not-array">
- <output-dir compare="Text">dimension-not-array</output-dir>
- <expected-error>CLUSTER BY 'dimension' must be an array of positive integers</expected-error>
+ <compilation-unit name="dimension-wrong-type">
+ <output-dir compare="Text">dimension-wrong-type</output-dir>
+ <expected-error>CLUSTER BY 'dimension' must be a positive integer or an array of positive integers</expected-error>
</compilation-unit>
</test-case>
<test-case FilePath="cluster-by/negative">
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/kmeans-syntax/kmeans-syntax.4.query.sqlpp
similarity index 69%
copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp
copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/kmeans-syntax/kmeans-syntax.4.query.sqlpp
index 09f4ede..59373f9 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/kmeans-syntax/kmeans-syntax.4.query.sqlpp
@@ -16,8 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-
-from [{"x": [1.0, 2.0]}] as r
-cluster by r.x as sc
+use test;
+-- "dimension" as a plain positive integer: k-means clusters a single field, so the array always holds
+-- exactly one element and the scalar spelling means the same as query 3's "dimension": [2].
+from Points as p
+cluster by p.vec as sc
+cluster as members
with { "clustering_algorithm": "K-Means", "dimension": 2, "num_clusters": 2 }
-select sc.cluster_id;
+select sc.cluster_id as cid,
+ array_count(members) as cnt,
+ array_count(sc.centroid) as dim
+order by cid;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-wrong-type/test.1.query.sqlpp
similarity index 91%
rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp
rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-wrong-type/test.1.query.sqlpp
index 09f4ede..697a9af 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-not-array/test.1.query.sqlpp
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/cluster-by/negative/dimension-wrong-type/test.1.query.sqlpp
@@ -19,5 +19,5 @@
from [{"x": [1.0, 2.0]}] as r
cluster by r.x as sc
-with { "clustering_algorithm": "K-Means", "dimension": 2, "num_clusters": 2 }
+with { "clustering_algorithm": "K-Means", "dimension": "2", "num_clusters": 2 }
select sc.cluster_id;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/cluster-by/kmeans-syntax/kmeans-syntax.4.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/cluster-by/kmeans-syntax/kmeans-syntax.4.adm
new file mode 100644
index 0000000..9d021eb
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/cluster-by/kmeans-syntax/kmeans-syntax.4.adm
@@ -0,0 +1,2 @@
+{ "cid": 0, "cnt": 3, "dim": 2 }
+{ "cid": 1, "cnt": 3, "dim": 2 }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppClusterByVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppClusterByVisitor.java
index 835e312..1132dac 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppClusterByVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppClusterByVisitor.java
@@ -436,7 +436,7 @@
/**
* Resolves the WITH record into the finished {@link ClusterByOptions}: one walk collects the raw values,
* rejecting unknown keys, then each option is validated in an order that lets every error name its own
- * option. {@code dimension} stays a raw node (it is an array); the scalar options flatten to strings.
+ * option. {@code dimension} stays a raw node (it may be an array); the scalar options flatten to strings.
*/
private ClusterByOptions resolveOptions(ClusterbyClause cbc) throws CompilationException {
SourceLocation loc = cbc.getSourceLocation();
@@ -606,30 +606,37 @@
* <p>
* An array, so that clustering on several fields can declare one width each. How many it must hold is the
* algorithm's to say: k-means clusters a single field (the grammar admits only one clustering expression),
- * so it allows exactly one.
+ * so it allows exactly one. Since that array always holds exactly one element, a plain positive integer is
+ * accepted as well and means the same thing: {@code "dimension": 384} is {@code "dimension": [384]}.
*/
private static int dimensionOf(IAdmNode node, SourceLocation loc) throws CompilationException {
if (node == null) {
throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
- "CLUSTER BY requires the 'dimension' option: the width of each clustering vector, as an "
- + "array, e.g. \"dimension\": [384].");
+ "CLUSTER BY requires the 'dimension' option: the width of each clustering vector, as a "
+ + "positive integer or a one-element array, e.g. \"dimension\": 384.");
}
- if (node.getType() != ATypeTag.ARRAY) {
- throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
- "CLUSTER BY 'dimension' must be an array of positive integers, e.g. [384].");
+ IAdmNode dimNode;
+ if (node.getType() == ATypeTag.ARRAY) {
+ AdmArrayNode dims = (AdmArrayNode) node;
+ if (dims.size() != 1) {
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
+ "CLUSTER BY with K-Means clusters a single field, so 'dimension' must hold exactly one "
+ + "element, but held " + dims.size() + ".");
+ }
+ dimNode = dims.get(0);
+ if (dimNode.getType() != ATypeTag.BIGINT) {
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
+ "CLUSTER BY 'dimension' must contain integers, but contained " + dimNode.getType() + ".");
+ }
+ } else {
+ dimNode = node;
+ if (dimNode.getType() != ATypeTag.BIGINT) {
+ throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
+ "CLUSTER BY 'dimension' must be a positive integer or an array of positive integers, "
+ + "e.g. 384 or [384], but was: " + dimNode.getType() + ".");
+ }
}
- AdmArrayNode dims = (AdmArrayNode) node;
- if (dims.size() != 1) {
- throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
- "CLUSTER BY with K-Means clusters a single field, so 'dimension' must hold exactly one "
- + "element, but held " + dims.size() + ".");
- }
- IAdmNode first = dims.get(0);
- if (first.getType() != ATypeTag.BIGINT) {
- throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
- "CLUSTER BY 'dimension' must contain integers, but contained " + first.getType() + ".");
- }
- long dim = ((AdmBigIntNode) first).get();
+ long dim = ((AdmBigIntNode) dimNode).get();
if (dim <= 0 || dim > Integer.MAX_VALUE) {
throw new CompilationException(ErrorCode.COMPILATION_ERROR, loc,
"CLUSTER BY 'dimension' must be a positive integer, but was: " + dim + ".");
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21790?usp=email
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: Ifa0ada68e905fb97fd4256aa98f9cd75af830414
Gerrit-Change-Number: 21790
Gerrit-PatchSet: 1
Gerrit-Owner: Shahrzad Shirazi <sh...@ucr.edu>