You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@graphar.apache.org by "keksmd (via GitHub)" <gi...@apache.org> on 2026/09/01 22:03:43 UTC
[PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
keksmd opened a new pull request, #972:
URL: https://github.com/apache/incubator-graphar/pull/972
Adds the reader side: `PhysicalReader`, `ReadReport`, `ReadResult`, plus a contract test that pins the invariant a physical reader must hold — a reader that declines every pushdown hint returns the same rows and the same schema as one that applies all of them. Only the reported `applied`/`declined` sets differ.
Builds on #971 (this PR's base branch). Together with the merged #961 (batch/write shapes) and #971 (request shapes), this completes the read half of the IO API.
Focused verify: `mvn --no-transfer-progress -f maven-projects/pom.xml -pl io-api -am spotless:check` and `clean verify -Dspotless.check.skip=true` — clean, 63 tests green.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on code in PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#discussion_r4023577804
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/ReadReport.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.Objects;
+import java.util.Set;
+
+/** Per-read accounting of physical hints applied or declined by a backend. */
+public final class ReadReport {
Review Comment:
Done: `ReadReport.equals/hashCode` over applied and declined sets.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "SemyonSinchenko (via GitHub)" <gi...@apache.org>.
SemyonSinchenko commented on code in PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#discussion_r4017727310
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
+ private final ComparisonOperator operator;
+ private final Literal value;
+
+ private Filter(String column, ComparisonOperator operator, Literal value) {
+ if (column == null || column.isBlank()) {
+ throw new IllegalArgumentException("A filter column cannot be blank.");
+ }
+ this.column = column;
+ this.operator = Objects.requireNonNull(operator, "Filter operator cannot be null.");
+ if ((operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL)
+ && value != null) {
+ throw new IllegalArgumentException(operator + " does not accept a comparison value.");
+ }
+ if (operator != ComparisonOperator.IS_NULL
+ && operator != ComparisonOperator.IS_NOT_NULL
+ && value == null) {
+ throw new IllegalArgumentException(operator + " requires a non-null comparison value.");
+ }
+ this.value = value;
+ }
+
+ /** Creates a filter with an immutable scalar comparison value. */
+ public static Filter comparison(String column, ComparisonOperator operator, Literal value) {
+ if (operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL) {
+ throw new IllegalArgumentException("Use isNull or isNotNull for null checks.");
+ }
+ return new Filter(column, operator, value);
+ }
+
+ /** Creates a null check for {@code column}. */
+ public static Filter isNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NULL, null);
+ }
+
+ /** Creates a non-null check for {@code column}. */
+ public static Filter isNotNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NOT_NULL, null);
+ }
+
Review Comment:
Why not add `Filter.gt`, `Filter.get`, `Filter.neq`, `Filter.lt`, `Filter.leq`, etc. and make the `public static Filter comparison(String column, ComparisonOperator operator, Literal value)` private internal contract?
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
+ private final ComparisonOperator operator;
+ private final Literal value;
+
+ private Filter(String column, ComparisonOperator operator, Literal value) {
+ if (column == null || column.isBlank()) {
+ throw new IllegalArgumentException("A filter column cannot be blank.");
+ }
+ this.column = column;
+ this.operator = Objects.requireNonNull(operator, "Filter operator cannot be null.");
+ if ((operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL)
+ && value != null) {
Review Comment:
Should we have `value` as `Option` instead of an explicit `null` contract? Passing `null` for optional value sounds like something from 2010th for me :D
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
Review Comment:
Should we make compositions possible, like
```java
var filter1 = Filter.isNotNull("a);
var filter2 = Filter.comparison("a", ComparisonOperator.GREATER_THAN, Literal.of(0.0));
var filter = filter1 && filter2;
```
??
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
+ private final ComparisonOperator operator;
+ private final Literal value;
+
+ private Filter(String column, ComparisonOperator operator, Literal value) {
+ if (column == null || column.isBlank()) {
+ throw new IllegalArgumentException("A filter column cannot be blank.");
+ }
+ this.column = column;
+ this.operator = Objects.requireNonNull(operator, "Filter operator cannot be null.");
+ if ((operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL)
+ && value != null) {
+ throw new IllegalArgumentException(operator + " does not accept a comparison value.");
+ }
+ if (operator != ComparisonOperator.IS_NULL
+ && operator != ComparisonOperator.IS_NOT_NULL
+ && value == null) {
+ throw new IllegalArgumentException(operator + " requires a non-null comparison value.");
+ }
+ this.value = value;
+ }
+
+ /** Creates a filter with an immutable scalar comparison value. */
+ public static Filter comparison(String column, ComparisonOperator operator, Literal value) {
+ if (operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL) {
+ throw new IllegalArgumentException("Use isNull or isNotNull for null checks.");
+ }
+ return new Filter(column, operator, value);
+ }
+
+ /** Creates a null check for {@code column}. */
+ public static Filter isNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NULL, null);
+ }
+
+ /** Creates a non-null check for {@code column}. */
+ public static Filter isNotNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NOT_NULL, null);
+ }
+
Review Comment:
In that case we can keep `null` contract and whatever we want, while users will get a clear and intuitive API...
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
Review Comment:
I like the idea of having `Literal` instead of `Object`, but the very logical next step will be to have `Column` instead of `String column`: by the end, filter can be a complex one, can't it?
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/ReadRequest.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.graphar.io;
+
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.OptionalLong;
+import java.util.Set;
+
+/**
+ * An immutable physical read operation. Filters are combined with logical AND and may require a
+ * backend to read columns that are not in the requested output projection.
+ */
+public final class ReadRequest {
Review Comment:
Let's add `equals` / `hashCode`, so the physical execution layer can, for example, deduplicate requests / cache results per request / etc.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Projection.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.graphar.io;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+/** An ordered set of requested output columns. */
+public final class Projection {
+ private static final Projection ALL_COLUMNS = new Projection(true, List.of());
+
+ private final boolean allColumns;
+ private final List<String> columns;
Review Comment:
I still do not like the idea of working with `String column`. I see a lot of potential risks, like unresolved vs resolved, unqualified vs qualified, etc. etc. As well possible collisions by unqualified names as an example.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/ReadReport.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.Objects;
+import java.util.Set;
+
+/** Per-read accounting of physical hints applied or declined by a backend. */
+public final class ReadReport {
Review Comment:
I would add `equals` / `hashCode`, so the system can, for example, separate processed / unprocessed and compare them elementwise or use `Set`
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5693765799
Addressed every thread above. Because the files live in the #971 commit this PR stacks on, the changes landed there (`ColumnRef`, `Filter` as a predicate tree with `Visitor`, named factories, `Optional` value, `Schema.resolve`, `ReadRequest.equals/hashCode`) and this PR was rebased on top with `ReadReport.equals/hashCode`. Locally `mvn -pl io-api -am clean verify`: #971 63 tests, #972 74 tests, 0 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: commits-unsubscribe@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on code in PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#discussion_r4023576090
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
+ private final ComparisonOperator operator;
+ private final Literal value;
+
+ private Filter(String column, ComparisonOperator operator, Literal value) {
+ if (column == null || column.isBlank()) {
+ throw new IllegalArgumentException("A filter column cannot be blank.");
+ }
+ this.column = column;
+ this.operator = Objects.requireNonNull(operator, "Filter operator cannot be null.");
+ if ((operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL)
+ && value != null) {
Review Comment:
Done: `Filter.Comparison.value()` returns `Optional<Literal>`, absent for null checks. The constructor is private and each factory builds exactly the shape it names, so there is no null contract left on the public surface.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
+ private final ComparisonOperator operator;
+ private final Literal value;
+
+ private Filter(String column, ComparisonOperator operator, Literal value) {
+ if (column == null || column.isBlank()) {
+ throw new IllegalArgumentException("A filter column cannot be blank.");
+ }
+ this.column = column;
+ this.operator = Objects.requireNonNull(operator, "Filter operator cannot be null.");
+ if ((operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL)
+ && value != null) {
+ throw new IllegalArgumentException(operator + " does not accept a comparison value.");
+ }
+ if (operator != ComparisonOperator.IS_NULL
+ && operator != ComparisonOperator.IS_NOT_NULL
+ && value == null) {
+ throw new IllegalArgumentException(operator + " requires a non-null comparison value.");
+ }
+ this.value = value;
+ }
+
+ /** Creates a filter with an immutable scalar comparison value. */
+ public static Filter comparison(String column, ComparisonOperator operator, Literal value) {
+ if (operator == ComparisonOperator.IS_NULL || operator == ComparisonOperator.IS_NOT_NULL) {
+ throw new IllegalArgumentException("Use isNull or isNotNull for null checks.");
+ }
+ return new Filter(column, operator, value);
+ }
+
+ /** Creates a null check for {@code column}. */
+ public static Filter isNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NULL, null);
+ }
+
+ /** Creates a non-null check for {@code column}. */
+ public static Filter isNotNull(String column) {
+ return new Filter(column, ComparisonOperator.IS_NOT_NULL, null);
+ }
+
Review Comment:
Done: `Filter.equal / notEqual / lessThan / lessThanOrEqual / greaterThan / greaterThanOrEqual / isNull / isNotNull`. `comparison(...)` is private now.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
+ private final String column;
Review Comment:
Done: `ColumnRef` replaces `String column` in both `Filter` and `Projection`, and `Filter` is now a predicate tree (see the thread on composition). I went with `ColumnRef` rather than `Column` because `RecordBatch.column(int)` already returns a vector and the two would read as the same thing.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Filter.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.graphar.io;
+
+import java.util.Objects;
+
+/**
+ * A single inspectable table-filter hint; request filters are combined with logical AND. A physical
+ * reader must validate a comparison literal against the column's {@link ColumnType}: BOOLEAN uses
+ * Boolean; integer and floating kinds use their matching boxed Java types; STRING uses String; DATE
+ * uses LocalDate; and TIMESTAMP_MILLIS uses millisecond-precise Instant. Other types cannot be
+ * compared by this contract.
+ *
+ * <p>Comparison operands must have the same declared type; a type mismatch is invalid rather than a
+ * coercion. Null values never match a comparison, including NOT_EQUAL; use IS_NULL or IS_NOT_NULL
+ * for null tests. Ordered STRING comparisons use {@link String#compareTo(String)}, and DATE and
+ * TIMESTAMP_MILLIS use their natural ordering. Readers must reject an invalid comparison before
+ * returning a result, whether the filter is pushed down or evaluated as fallback.
+ */
+public final class Filter {
Review Comment:
Done. `Filter` is a closed tree: `Comparison`, `And`, `Or`, `Not`. Static `Filter.and(a, b, ...)`, `Filter.or(...)`, `Filter.not(x)` plus instance `a.and(b)`, `a.or(b)`, `a.negate()`, so your example is `Filter.isNotNull(a).and(Filter.greaterThan(a, Literal.of(0.0)))`. Same-kind junctions flatten and a double negation collapses, so structurally equal predicates compare equal. A reader walks the tree with `Filter.Visitor<R>` - one callback per node kind - so a new node kind is a compile error in every backend rather than a silently skipped branch. `ReadRequest.filters()` stays a list; it is still the top-level conjunction so a backend can push some terms down and evaluate the rest.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/Projection.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.graphar.io;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+/** An ordered set of requested output columns. */
+public final class Projection {
+ private static final Projection ALL_COLUMNS = new Projection(true, List.of());
+
+ private final boolean allColumns;
+ private final List<String> columns;
Review Comment:
Agreed, and this round moves the boundary to a typed `ColumnRef`. On the specific risks: a request is built before its file is opened, so a reference is unresolved by construction; the only place it binds is `Schema.resolve(ColumnRef) -> int` in the backend. That resolution is exact - verbatim name match, unknown column is an error, and a name that matches more than one field is an error (`Schema` allows duplicates, as you noted on #961, so first-match would be silent data corruption). There is no qualifier on purpose: the physical boundary reads one file with one schema, and mapping a qualified logical property onto a physical column is the job of the layer above. Nested paths are not in `ColumnType` yet, so `ColumnRef` is one segment today and grows to a path without breaking callers when a struct kind lands.
##########
maven-projects/io-api/src/main/java/org/apache/graphar/io/ReadRequest.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.graphar.io;
+
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.OptionalLong;
+import java.util.Set;
+
+/**
+ * An immutable physical read operation. Filters are combined with logical AND and may require a
+ * backend to read columns that are not in the requested output projection.
+ */
+public final class ReadRequest {
Review Comment:
Done: `ReadRequest.equals/hashCode` over uri, projection, row range, filters and limit, with a test that flips each one.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5601476528
@SemyonSinchenko this is the other half of the read contract — a backend reports which hints it actually applied, so a declined pushdown is visible rather than silently changing nothing.
Green and rebased on current `main`. Best read after #971, which it sits on top of.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5566666341
@SemyonSinchenko @SYaoJun this one builds on #971, so please review that first. It is opened against `main` because GitHub cannot base a pull request on a branch that lives in a fork, so the diff currently contains the #971 commits as well; it will shrink to its own slice once #971 lands.
The slice itself makes a declined pushdown observable: a backend reports which hints it applied, and declining one must not change the results.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "SemyonSinchenko (via GitHub)" <gi...@apache.org>.
SemyonSinchenko merged PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "SemyonSinchenko (via GitHub)" <gi...@apache.org>.
SemyonSinchenko commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5694684166
Merging this because there are no risks: Java module is not published yet.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5501046861
cc @SemyonSinchenko @SYaoJun — stacked on #971 (shows its commits until that merges, diff will shrink automatically). Green CI.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org
Re: [PR] feat(java): make a declined pushdown visible instead of silent [incubator-graphar]
Posted by "keksmd (via GitHub)" <gi...@apache.org>.
keksmd commented on PR #972:
URL: https://github.com/apache/incubator-graphar/pull/972#issuecomment-5601281546
Rebased onto current `main` (now including #966 and #968) and force-pushed. Locally `mvn -pl io-api -am clean verify` gives 63 tests, 0 failures, 0 errors, 0 skipped.
This one sits on top of #971 — reviewing and merging #971 first leaves a single commit here. Together the two are the last piece the Parquet backend in #964 needs, so I am holding that PR until both land rather than opening a dependent chain.
--
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@graphar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org