pgwhalen commented on code in PR #119: URL: https://github.com/apache/datafusion-java/pull/119#discussion_r4437303426
########## core/src/main/java/org/apache/datafusion/TableScanRequest.java: ########## @@ -0,0 +1,47 @@ +/* + * 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.datafusion; + +import java.util.List; +import java.util.Optional; +import java.util.OptionalLong; + +import org.apache.datafusion.protobuf.LogicalExprNode; + +/** + * A physical scan request using DataFusion's projection, expression and limit semantics. + * + * <p>Projection indices refer to the registered table schema. An absent projection means all + * columns; a present empty list means no columns (for example, {@code COUNT(*)}), not no rows. + * Filters use the same expression protobufs as DataFusion logical plans, not SQL strings. + * + * <p>The optional limit is the value supplied by DataFusion's planner. Sources may return more + * rows; they must not return fewer than requested when that many matching rows are available. + * DataFusion retains responsibility for deciding whether a limit can safely cross a residual + * filter. Limits outside the non-negative Java {@code long} range are not forwarded; DataFusion + * still applies the query's limit. + */ +public record TableScanRequest( Review Comment: I believe this corresponds to `ScanArgs` in rust so you could rename it to that. ########## core/src/main/java/org/apache/datafusion/ScanTableProvider.java: ########## @@ -0,0 +1,73 @@ +/* + * 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.datafusion; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.OptionalLong; + +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.vector.ipc.ArrowReader; +import org.apache.datafusion.protobuf.LogicalExprNode; + +/** + * Optional scan planning and source pushdown for a Java-implemented {@link TableProvider}. + * + * <p>The Rust adapter forwards DataFusion's scan request without interpreting source predicates. + * Implementations choose their own access path and return a lazy {@link TableScan}. Its readers + * must produce the requested projected schema, including a zero-column schema when requested. + * + * <p>Planning and capability callbacks may run more than once and must not consume source rows. + * Sources that cannot execute a requested access path may reject it during planning rather than + * opening an unwanted unbounded scan. + * + * <p>Callbacks may run on native worker threads. Shared provider state and reader factories must be + * thread-safe; each execution owns an independent reader. + */ +public interface ScanTableProvider extends TableProvider { + /** DataFusion's per-filter pushdown contract. */ + enum FilterPushdown { + /** The source does not apply this filter; DataFusion keeps it above the scan. */ + UNSUPPORTED, + /** The source prunes candidates; DataFusion also applies the filter to the returned rows. */ + INEXACT, + /** The source completely applies the filter with DataFusion-compatible semantics. */ + EXACT + } + + /** + * Return one capability per input filter, in the same order. The default declines every filter. + * An inexact filter must never discard a matching row. + */ + default List<FilterPushdown> supportsFiltersPushdown(List<LogicalExprNode> filters) { + return Collections.nCopies(filters.size(), FilterPushdown.UNSUPPORTED); + } + + /** Plan a source scan without reading rows. Only supported/inexact filters are included. */ + TableScan planScan(TableScanRequest request); Review Comment: If the other changes re: matching rust structure are made, this would become `scanWithArgs`. ########## core/src/main/java/org/apache/datafusion/ScanTableProvider.java: ########## @@ -0,0 +1,73 @@ +/* + * 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.datafusion; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.OptionalLong; + +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.vector.ipc.ArrowReader; +import org.apache.datafusion.protobuf.LogicalExprNode; + +/** + * Optional scan planning and source pushdown for a Java-implemented {@link TableProvider}. + * + * <p>The Rust adapter forwards DataFusion's scan request without interpreting source predicates. + * Implementations choose their own access path and return a lazy {@link TableScan}. Its readers + * must produce the requested projected schema, including a zero-column schema when requested. + * + * <p>Planning and capability callbacks may run more than once and must not consume source rows. + * Sources that cannot execute a requested access path may reject it during planning rather than + * opening an unwanted unbounded scan. + * + * <p>Callbacks may run on native worker threads. Shared provider state and reader factories must be + * thread-safe; each execution owns an independent reader. + */ +public interface ScanTableProvider extends TableProvider { Review Comment: What do you think about adding these as default methods on the `TableProvider` interface to match the rust trait? Then the `planScan` (potentially renamed to `scanWithArgs`) could just delegate to `scan`. ########## core/src/main/java/org/apache/datafusion/TableScan.java: ########## @@ -0,0 +1,38 @@ +/* + * 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.datafusion; + +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.vector.ipc.ArrowReader; + +/** + * A bounded, single-partition leaf scan planned by a {@link ScanTableProvider}. + * + * <p>Constructing the scan must not read source rows. Each execution opens a fresh independent + * reader whose schema is the requested projection of the registered table schema. Reader buffers + * must use the supplied allocator or a child of it. DataFusion owns and closes the reader. + * + * <p>This is a Java-backed scan, not an API for arbitrary child-consuming physical operators. + */ +@FunctionalInterface +public interface TableScan { Review Comment: Since `TableScan` has another meaning in rust, could we rename this to `ExecutionPlan` which it seems to match? That's about how my own bindings do it. Could even make it take the partition index so future support for multi-partition plans (which I'd love to add) could be added without breaking compatibility. But maybe that sort of thing matters a lot less now in an agentic coding world 🤔 . -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
