You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "nva (via GitHub)" <gi...@apache.org> on 2023/11/27 10:29:39 UTC
[PR] IGNITE-20878 Added basic criteria queries. [ignite-3]
nva opened a new pull request, #2882:
URL: https://github.com/apache/ignite-3/pull/2882
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427903249
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Represents a predicate for performing criteria queries.
+ */
+public interface Criteria {
Review Comment:
Why is it empty? Is it just a preparation to future implementation?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434783027
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItCriteriaQueryTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.ignite.internal.table;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgnitionManager;
+import org.apache.ignite.InitParameters;
+import org.apache.ignite.internal.lang.IgniteStringFormatter;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.testframework.TestIgnitionManager;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests for the criteria query table API.
+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItCriteriaQueryTest extends BaseIgniteAbstractTest {
+ private static final String TABLE_NAME = "SOME_TABLE";
+
+ private static final String COLUMN_KEY = "key";
+
+ private static final int BASE_PORT = 3344;
+
+ private static final String NODE_BOOTSTRAP_CFG = "{\n"
Review Comment:
Used `ClusterPerClassIntegrationTest` as base class
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432708678
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
Review Comment:
Replaced with view
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434145652
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
I would say `Async` specify purpose of the adapter, but it should looks more right like `CursorSyncAdaptar`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1424102893
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.ignite.sql;
+
+import static java.util.stream.Collectors.toList;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this cursor.
+ *
+ * @return Sequential Stream over the elements covered by this cursor.
+ */
+ default Stream<T> stream() {
+ return StreamSupport.stream(Spliterators.spliteratorUnknownSize(this, 0), false);
+ }
+
+ /**
+ * Gets all query results and stores them in the collection.
+ * Use this method when you know in advance that query result is
+ * relatively small and will not cause memory utilization issues.
+ *
+ * <p>Since all the results will be fetched, all the resources will be closed
Review Comment:
The garants is applicable for default implementation?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434782782
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItCriteriaQueryTest.java:
##########
Review Comment:
Moved to `table` module
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -357,6 +361,7 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
* @param raftGroupServiceFactory Factory that is used for creation of raft group services for replication groups.
* @param vaultManager Vault manager.
* @param placementDriver Placement driver.
+ * @param sql Ignite SQL facade.
Review Comment:
Fixed javadoc
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429652088
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
+ * @see KeyValueView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
Review Comment:
So, it can be `this`, it can be error. Right now I see just one mentioned error. If we look into implementation we can see different errors for different cases (possibly it wrong, but when we change interfaces, why we don't check and align implementation and documentation?).
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#issuecomment-1846864371
> I'd suggest to split the PR into 2 ones.
>
> 1. Criteria API implementation - criteria builders and all the stuff, which is used to build criteria objects and generates SQL query from them.
> 2. CriteriaQuery API implementation - all the `criteriaQuery` methods, which accepts Criteria, and cursors, and the workflow stuff (Here, you may need an empty Criteria interface with `toString()` method)
Move Criteria API implementation into #2933
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434126825
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
Removed. Another good candidate for this `SyncResultSetAdapter`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432751978
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +392,36 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ //TODO: implement serialization of criteria to SQL https://issues.apache.org/jira/browse/IGNITE-20879
+ var query = "SELECT * FROM " + tbl.name();
+
+ Statement statement = new ClientStatementBuilder().query(query).pageSize(opts.pageSize()).build();
+ Session session = new ClientSessionBuilder(tbl.channel()).build();
+
+ return session.executeAsync(tx, statement)
+ .thenApply(resultSet -> new QueryCriteriaAsyncResultSet<>(resultSet, session::close));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public ClosableCursor<Tuple> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts) {
+ return new SyncResultSetAdapter<>(executeAsync(tx, criteria, opts).join());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public CompletableFuture<AsyncClosableCursor<Tuple>> queryCriteriaAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ return executeAsync(tx, criteria, opts)
+ .thenApply(Function.identity());
Review Comment:
redundant action and could be deleted.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434197131
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItCriteriaQueryTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.ignite.internal.table;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgnitionManager;
+import org.apache.ignite.InitParameters;
+import org.apache.ignite.internal.lang.IgniteStringFormatter;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.testframework.TestIgnitionManager;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.Session;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests for the criteria query table API.
+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItCriteriaQueryTest extends BaseIgniteAbstractTest {
+ private static final String TABLE_NAME = "SOME_TABLE";
+
+ private static final String COLUMN_KEY = "key";
+
+ private static final int BASE_PORT = 3344;
+
+ private static final String NODE_BOOTSTRAP_CFG = "{\n"
Review Comment:
can it be used some of existing base integration tests instead of creating the noise with configuring cluster?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "agura (via GitHub)" <gi...@apache.org>.
agura commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1436056438
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
@ygerzhedovich Once again: not async means sync :)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407793925
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaBuilder.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria builder. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public class CriteriaBuilder implements Criteria {
+ private final List<Criteria> lsCriteria = new ArrayList<>();
+ private final String columnName;
+
+ private CriteriaBuilder(String columnName) {
+ this.columnName = columnName;
+ }
+
+ public static CriteriaBuilder columnName(String columnName) {
Review Comment:
What does the next could mean?
```
CriteriaBuilder.columnName('col1').equals(1).equals(2)
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1424130114
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
Review Comment:
what does it mean 'completed with the same object'?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427868516
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
+ * @see KeyValueView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
Review Comment:
it's not true, about returnin `this`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432623318
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
Review Comment:
Changed to `Executes a query to a view using a predicate.`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429986813
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to run a given query. Can be {@code null} to run within implicit transaction.
+ * @param criteria If {@code null} then all entries will be returned.
Review Comment:
What does mean `all entries`? That's `all` relates to what?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431374998
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
Review Comment:
it says just about `null` value, but nothing about what is it. Seems need to mention that is predicates which you mention often in the interface
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434194203
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItCriteriaQueryTest.java:
##########
Review Comment:
seems all tests related to different part of system, but not to runner module. Let's place all test to corresponding modules
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407796044
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaBuilder.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria builder. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public class CriteriaBuilder implements Criteria {
+ private final List<Criteria> lsCriteria = new ArrayList<>();
+ private final String columnName;
+
+ private CriteriaBuilder(String columnName) {
+ this.columnName = columnName;
+ }
+
+ public static CriteriaBuilder columnName(String columnName) {
Review Comment:
Let's make Criteria and builder a separate classes/interfaces.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407817741
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/StaticText.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code StaticText} represents a static text element.
+ */
+public class StaticText implements Expression {
Review Comment:
Why don't use `Constant` instead?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429968246
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
Review Comment:
AI3 doesn't have cache, just tables
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432709529
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to run a given query. Can be {@code null} to run within implicit transaction.
+ * @param criteria If {@code null} then all entries will be returned.
Review Comment:
Related to view
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432623926
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
Review Comment:
Changed to `Transaction to execute the query within or {@code null} to run within implicit transaction.`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432703852
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQueryOptions.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Criteria query options.
Review Comment:
expanded the description
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407791416
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaBuilder.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria builder. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public class CriteriaBuilder implements Criteria {
+ private final List<Criteria> lsCriteria = new ArrayList<>();
+ private final String columnName;
+
+ private CriteriaBuilder(String columnName) {
+ this.columnName = columnName;
+ }
+
+ public static CriteriaBuilder columnName(String columnName) {
Review Comment:
The builder pattern supposes `build()` method is called at last, when all the work is done, but there is no one.
Regarding this and a fact the object `implements Criteria`, I guess the builder is always in valid state... but looking at the implementation - I think it is not true.
What if a user creates a builder without calling `equals(..)` method
```
CriteriaBuilder.columnName('col1')
```
Will the builder be in valid state?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407808585
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Expression.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code Expression} defines a general typed expression in a Query instance. The generic type parameter
+ * is a reference to the type the expression is bound to.
+ */
+public interface Expression {
+ /**
+ * Accept the visitor with the given context.
+ *
+ * @param <R> return type
+ * @param <C> context type
+ * @param v visitor
+ * @param context context of visit
+ */
+ <R, C> void accept(CriteriaVisitor<C> v, @Nullable C context);
Review Comment:
`R` is unused. There is no return type.
```suggestion
<C> void accept(CriteriaVisitor<C> v, @Nullable C context);
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1436813131
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
@agura , in general I agree with it, but it looks more suitable when you can understand purpose of adapter. Adapter for what reason?
BTW, current variant also 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427876624
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQueryOptions.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Criteria query options.
Review Comment:
it's a public API - will be better to have more wide Javadoc
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1425067470
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
Review Comment:
This javadoc was moved from `AsyncResultSet#fetchNextPage()`. This means that future will return `this`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407817215
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Constant.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code Constant} represents a general constant expression.
+ *
+ * @param <T> constant type
+ */
+public class Constant<T> implements Expression {
+ private final T constant;
+
+ /**
+ * Create a new Constant of the given type for the given object.
+ *
+ * @param constant wrapped constant.
+ */
+ Constant(T constant) {
+ this.constant = constant;
+ }
+
+ /**
+ * Get the wrapped constant.
+ *
+ * @return wrapped constant
+ */
+ public T getConstant() {
+ return constant;
+ }
Review Comment:
`Constant` is an `Expression` in the type system.
So, here method name look a bit confusing.
```suggestion
/**
* Gets constant value.
*
* @return A value.
*/
public T getValue() {
return valuet;
}
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1433750039
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +392,36 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
Review Comment:
Method has been removed and all logic has been moved to `queryCriteriaAsync`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#issuecomment-1829959968
I'd suggest to split the PR into 2 ones.
1. Criteria API implementation - criteria builders and all the stuff, which is used to build criteria objects and generates SQL query from them.
2. CriteriaQuery API implementation - all the `criteriaQuery` methods, which accepts Criteria, and cursors, and the workflow stuff (Here, you may need an empty Criteria interface with `toString()` method)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1441442375
##########
modules/api/src/main/java/org/apache/ignite/lang/AsyncCursor.java:
##########
@@ -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.
+ */
+
+package org.apache.ignite.lang;
+
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * Provides methods for iterate over operation results and release underlying resources in an asynchronous way.
Review Comment:
```suggestion
* Provides methods to iterate over operation results and release underlying resources in an asynchronous way.
```
##########
modules/api/src/main/java/org/apache/ignite/lang/Cursor.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.ignite.lang;
+
+import java.util.Iterator;
+
+/**
+ * Provides methods for iterate over operation results and release underlying resources.
Review Comment:
```suggestion
* Provides methods to iterate over operation results and release underlying resources.
```
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
Review Comment:
```suggestion
* Executes a criteria query.
```
I don't think "record view" should be mentioned - we might add this to KeyValueViews as well.
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
Review Comment:
Same as above, here and below - let's not mention "record view".
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Represents a predicate for performing criteria queries.
Review Comment:
```suggestion
* Represents a criteria query predicate.
```
##########
modules/api/src/main/java/org/apache/ignite/lang/Cursor.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.ignite.lang;
+
+import java.util.Iterator;
+
+/**
+ * Provides methods for iterate over operation results and release underlying resources.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ */
+public interface Cursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Closes this resource and releasing any underlying resources.
Review Comment:
```suggestion
* Closes this cursor and releases any underlying resources.
```
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
Review Comment:
```suggestion
* Represents an object that can be used to execute predicate-based criteria queries.
```
##########
modules/api/src/main/java/org/apache/ignite/sql/ResultSet.java:
##########
@@ -70,12 +70,12 @@ public interface ResultSet<T> extends Iterator<T>, AutoCloseable {
long affectedRows();
/**
- * Indicated whether the query that had produced the result was a conditional query.
- * E.g., for query "Create table if not exists", the method returns {@code true} if
+ * Indicated whether the query that had produced the result was a conditional query.
Review Comment:
```suggestion
* Indicates whether the query that had produced the result was a conditional query.
```
##########
modules/api/src/main/java/org/apache/ignite/lang/AsyncCursor.java:
##########
@@ -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.
+ */
+
+package org.apache.ignite.lang;
+
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * Provides methods for iterate over operation results and release underlying resources in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ */
+public interface AsyncCursor<T> {
+ /**
+ * Returns the current page content if the operation returns results.
+ *
+ * @return Iterable set of elements.
+ * @throws IgniteException If no results is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the operation return results.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws IgniteException If no results is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
+ * @throws IgniteException If resource is closed or if there are no more results.
+ */
+ CompletableFuture<? extends AsyncCursor<T>> fetchNextPage();
+
+ /**
+ * Indicates whether there are more pages of results.
+ *
+ * @return {@code True} if there are more pages with results, {@code false} otherwise.
+ */
+ boolean hasMorePages();
+
+ /**
+ * Closes this resource and releasing any underlying resources.
Review Comment:
```suggestion
* Closes this cursor and releases any underlying resources.
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1425079227
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.ignite.sql;
+
+import static java.util.stream.Collectors.toList;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this cursor.
+ *
+ * @return Sequential Stream over the elements covered by this cursor.
+ */
+ default Stream<T> stream() {
+ return StreamSupport.stream(Spliterators.spliteratorUnknownSize(this, 0), false);
+ }
+
+ /**
+ * Gets all query results and stores them in the collection.
+ * Use this method when you know in advance that query result is
+ * relatively small and will not cause memory utilization issues.
+ *
+ * <p>Since all the results will be fetched, all the resources will be closed
+ * automatically after this call, e.g. there is no need to call {@link #close()} method in this case.
+ *
+ * @return List containing all query results.
+ */
+ default List<T> getAll() {
Review Comment:
This was copped from AI2`QueryCursor`. Removed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1424115854
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.ignite.sql;
+
+import static java.util.stream.Collectors.toList;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this cursor.
+ *
+ * @return Sequential Stream over the elements covered by this cursor.
+ */
+ default Stream<T> stream() {
+ return StreamSupport.stream(Spliterators.spliteratorUnknownSize(this, 0), false);
+ }
+
+ /**
+ * Gets all query results and stores them in the collection.
+ * Use this method when you know in advance that query result is
+ * relatively small and will not cause memory utilization issues.
+ *
+ * <p>Since all the results will be fetched, all the resources will be closed
+ * automatically after this call, e.g. there is no need to call {@link #close()} method in this case.
+ *
+ * @return List containing all query results.
+ */
+ default List<T> getAll() {
Review Comment:
Why you want to have suh method in the interface?
Potentially it could OOM, Also it looks as utility method to retrieve all results from a stream and shouldn't be a part of interface.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429723743
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -521,4 +523,16 @@ public static <T> Set<T> intersect(Set<T> op1, Set<T> op2) {
return Set.copyOf(set);
}
+
+ /**
+ * Gets a list based on an iterator.
+ *
+ * @param iterator The iterator to use.
+ * @return A list of the iterator contents.
+ * @throws NullPointerException if iterator parameter is null
+ */
+ public static <T> List<T> toList(Iterator<T> iterator) {
Review Comment:
As I see the conversion required just for tests. If it so it shouldn't be here. For tests exists org.junit.platform.commons.util.CollectionUtils.toStream and so 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429936766
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -521,4 +523,16 @@ public static <T> Set<T> intersect(Set<T> op1, Set<T> op2) {
return Set.copyOf(set);
}
+
+ /**
+ * Gets a list based on an iterator.
+ *
+ * @param iterator The iterator to use.
+ * @return A list of the iterator contents.
+ * @throws NullPointerException if iterator parameter is null
+ */
+ public static <T> List<T> toList(Iterator<T> iterator) {
Review Comment:
This can be used by the customer to process the criteria query results, but let the customer choose what to use.
`org.junit.platform.commons.util.CollectionUtils.toStream` - Any usage by external parties is not supported
So I replaced it with `Lists#newArrayList`, which was available by default for tests
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
Review Comment:
Added
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1416630525
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/StaticText.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code StaticText} represents a static text element.
+ */
+public class StaticText implements Expression {
Review Comment:
Renamed `Constant` to `Argument`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432702787
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
Review Comment:
expanded the description
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432707110
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +388,34 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ //TODO: implement serialization of criteria to SQL https://issues.apache.org/jira/browse/IGNITE-20879
+ var query = "SELECT * FROM " + tbl.name();
+
+ var statement = tbl.sql().statementBuilder().query(query).pageSize(opts.pageSize()).build();
Review Comment:
Removed all incorrect uses of `var`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva merged PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1441219159
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/TupleMatcher.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.ignite.internal.testframework.matchers;
+
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Matchers for {@link Tuple}.
+ */
+public class TupleMatcher {
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1440451416
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return query(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @param opts Criteria query options.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
Review Comment:
it shouldn't be SqlException
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1415247864
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.table.criteria.CriteriaBuilder.columnName;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.criteria.Criteria;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).getAll();
+ assertThat(res, hasSize(3));
+
+ res = view.queryCriteria(null, columnName(COLUMN_KEY).equal(2)).getAll();
+ assertThat(res, hasSize(1));
+ assertThat(res, hasItem(tupleValue(COLUMN_KEY, equalTo(2))));
+
+ res = view.queryCriteria(null, Criteria.not(Criteria.equal(COLUMN_KEY, 2))).getAll();
Review Comment:
Done
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427891989
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##########
@@ -413,7 +415,8 @@ private PartialNode startPartialNode(
schemaSyncService,
catalogManager,
new HybridTimestampTracker(),
- placementDriverManager.placementDriver()
+ placementDriverManager.placementDriver(),
+ () -> mock(IgniteSql.class)
Review Comment:
As I see the test hadn't use mock before the patch. Why you decided to mix different approaches?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429942250
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -521,4 +523,16 @@ public static <T> Set<T> intersect(Set<T> op1, Set<T> op2) {
return Set.copyOf(set);
}
+
+ /**
+ * Gets a list based on an iterator.
+ *
+ * @param iterator The iterator to use.
+ * @return A list of the iterator contents.
+ * @throws NullPointerException if iterator parameter is null
+ */
+ public static <T> List<T> toList(Iterator<T> iterator) {
Review Comment:
It can't be used by customer due to it is internal utilities.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431366750
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.table.criteria.CriteriaQuerySource;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
Review Comment:
why we need to see `ClosableCursor`?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431670966
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.table.criteria.CriteriaQuerySource;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
Review Comment:
`ClosableCursor` as a synchronous variant of `AsyncClosableCursor`. Just like the `ResultSet` in the `AsyncResultSet'.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434209520
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -357,6 +361,7 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
* @param raftGroupServiceFactory Factory that is used for creation of raft group services for replication groups.
* @param vaultManager Vault manager.
* @param placementDriver Placement driver.
+ * @param sql Ignite SQL facade.
Review Comment:
it's not true
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434236099
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -381,7 +386,8 @@ public TableManager(
SchemaSyncService schemaSyncService,
CatalogService catalogService,
HybridTimestampTracker observableTimestampTracker,
- PlacementDriver placementDriver
+ PlacementDriver placementDriver,
+ Supplier<IgniteSql> sql
Review Comment:
We already have cyclic dependencies in `ComputeExecutorImpl`.
We can pass `Ignite` instance instead of `Supplier<IgniteSql>`. Will it look better?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434782782
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItCriteriaQueryTest.java:
##########
Review Comment:
Moved to `sql-engine` module
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1424123833
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return Operation future.
+ * @throws NoRowSetExpectedException if no row set is expected as a query result.
+ */
+ CompletableFuture<? extends AsyncResultSet<T>> fetchNextPage();
Review Comment:
It look very strange to return an interface that is lower in the hierarchy.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407782256
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaBuilder.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria builder. Implementations of this interface are basic building blocks for performing criteria queries.
Review Comment:
```suggestion
* Criteria builder implementation.
```
It worth to mention few examples in javadoc on how to use the builder.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427880783
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @param opts Criteria query options.
+ * @throws SqlException If failed.
+ */
+ ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts);
+
+ /**
+ * Execute criteria query over cache entries in an asynchronous way.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @return Operation future.
+ * @throws SqlException If failed.
+ */
+ default CompletableFuture<? extends AsyncClosableCursor<T>> queryCriteriaAsync(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteriaAsync(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Execute criteria query over cache entries in an asynchronous way.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
Review Comment:
what does mean null for transaction?
I suggest to review carefully all part related to API module - it's a face of our product
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1415250181
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaBuilder.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria builder. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public class CriteriaBuilder implements Criteria {
+ private final List<Criteria> lsCriteria = new ArrayList<>();
+ private final String columnName;
+
+ private CriteriaBuilder(String columnName) {
+ this.columnName = columnName;
+ }
+
+ public static CriteriaBuilder columnName(String columnName) {
Review Comment:
I decided to remove `CriteriaBuilder` from this PR at all. We can add it later. It was optional
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429945673
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
+ * @see KeyValueView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
Review Comment:
Expanded the throws section. It was not expected that I should double-check the API and its documentation.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429974768
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to run a given query. Can be {@code null} to run within implicit transaction.
Review Comment:
which `given` query?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434208578
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -342,7 +347,6 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
/**
* Creates a new table manager.
*
- * @param nodeName Node name.
Review Comment:
?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431379202
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
Review Comment:
I'm not sure that we should use table in the context. Can we use it for querying views for example?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434786181
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
Review Comment:
@agura Do I understand correctly that you are suggesting using `query(null, criteria, criteriaOpts)`?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1433750980
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +392,36 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ //TODO: implement serialization of criteria to SQL https://issues.apache.org/jira/browse/IGNITE-20879
+ var query = "SELECT * FROM " + tbl.name();
+
+ Statement statement = new ClientStatementBuilder().query(query).pageSize(opts.pageSize()).build();
+ Session session = new ClientSessionBuilder(tbl.channel()).build();
+
+ return session.executeAsync(tx, statement)
+ .thenApply(resultSet -> new QueryCriteriaAsyncResultSet<>(resultSet, session::close));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public ClosableCursor<Tuple> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts) {
+ return new SyncResultSetAdapter<>(executeAsync(tx, criteria, opts).join());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public CompletableFuture<AsyncClosableCursor<Tuple>> queryCriteriaAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ return executeAsync(tx, criteria, opts)
+ .thenApply(Function.identity());
Review Comment:
Removed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1433750039
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +392,36 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
Review Comment:
The separate method has been removed and all logic has been moved to `queryCriteriaAsync`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434213534
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -381,7 +386,8 @@ public TableManager(
SchemaSyncService schemaSyncService,
CatalogService catalogService,
HybridTimestampTracker observableTimestampTracker,
- PlacementDriver placementDriver
+ PlacementDriver placementDriver,
+ Supplier<IgniteSql> sql
Review Comment:
the approach say us the we have cyclic dependencies.... It looks bad
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1440451099
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return query(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @param opts Criteria query options.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts);
Review Comment:
we can use `null` for transaction and for criteria, but CriteriaQueryOptions still mandatory, it looks inconsistent.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407844685
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.table.criteria.CriteriaBuilder.columnName;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.criteria.Criteria;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).getAll();
+ assertThat(res, hasSize(3));
+
+ res = view.queryCriteria(null, columnName(COLUMN_KEY).equal(2)).getAll();
+ assertThat(res, hasSize(1));
+ assertThat(res, hasItem(tupleValue(COLUMN_KEY, equalTo(2))));
+
+ res = view.queryCriteria(null, Criteria.not(Criteria.equal(COLUMN_KEY, 2))).getAll();
Review Comment:
Do you mean literally 'column_key' variable value equals to integer of 2 ?
```
Criteria.not(Criteria.equal(COLUMN_KEY, 2)))
```
I like Hamcrest project API, whose approach could look more natural.
E.g.
```
Criteria.not(Criteria.columnValue(COLUMN_KEY, Expression.equalTo(2)))
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1425067470
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
Review Comment:
This javadoc was copped from `AsyncResultSet#fetchNextPage()`. This means that future will return `this`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427894510
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
Review Comment:
please check all places with `var` which were added in the patch.
[https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide#JavaCodeStyleGuide-3Usingvarkeyword](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide#JavaCodeStyleGuide-3Usingvarkeyword)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427903249
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Represents a predicate for performing criteria queries.
+ */
+public interface Criteria {
Review Comment:
Why is it empty? Is it just a preparation to future implementation?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1425067470
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
Review Comment:
This javadoc was moved from `AsyncResultSet#fetchNextPage()`. This means that future return `this`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429746671
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
Review Comment:
Fixed all places, but it's better to have a checkstyle rule for this.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429749001
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
+ RecordView<TestPojo> view = client().tables().table(TABLE_NAME).recordView(TestPojo.class);
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ private void populateData() {
+ var table = client().tables().table(TABLE_NAME).recordView();
+
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 1, COLUMN_VAL, "1")));
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 2, COLUMN_VAL, "2")));
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 3, COLUMN_VAL, "3")));
+ }
+
+ /**
+ * Creates a matcher for matching tuple value.
+ *
+ * @param valueMatcher Matcher for matching tuple value.
+ * @return Matcher for matching tuple value.
+ */
+ private static <T> Matcher<Tuple> tupleValue(String columnName, Matcher<T> valueMatcher) {
Review Comment:
Deleted. I will add this in another PR where it is used.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432753108
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +392,36 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
Review Comment:
let's rename it to more specific
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432699957
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.ignite.sql;
+
+import static java.util.stream.Collectors.toList;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this cursor.
+ *
+ * @return Sequential Stream over the elements covered by this cursor.
+ */
+ default Stream<T> stream() {
+ return StreamSupport.stream(Spliterators.spliteratorUnknownSize(this, 0), false);
+ }
+
+ /**
+ * Gets all query results and stores them in the collection.
+ * Use this method when you know in advance that query result is
+ * relatively small and will not cause memory utilization issues.
+ *
+ * <p>Since all the results will be fetched, all the resources will be closed
Review Comment:
the method has been deleted
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432624476
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.table.criteria.CriteriaQuerySource;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
Review Comment:
Removed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432701048
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
+ *
+ * @param <T> Type of elements.
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The future that is completed with the same {@code AsyncResultSet} object.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return Operation future.
+ * @throws NoRowSetExpectedException if no row set is expected as a query result.
+ */
+ CompletableFuture<? extends AsyncResultSet<T>> fetchNextPage();
Review Comment:
Added by mistake, fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434128635
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in view.
Review Comment:
Replaced with `record view`
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
Review Comment:
Replaced with `record view`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1440566922
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return query(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @param opts Criteria query options.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
Review Comment:
Changed to IgniteException
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1440682087
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/TupleMatcher.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.ignite.internal.testframework.matchers;
+
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Matchers for {@link Tuple}.
+ */
+public class TupleMatcher {
Review Comment:
```suggestion
public final class TupleMatcher {
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407836838
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.table.criteria.CriteriaBuilder.columnName;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.criteria.Criteria;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).getAll();
+ assertThat(res, hasSize(3));
+
+ res = view.queryCriteria(null, columnName(COLUMN_KEY).equal(2)).getAll();
+ assertThat(res, hasSize(1));
+ assertThat(res, hasItem(tupleValue(COLUMN_KEY, equalTo(2))));
Review Comment:
I don't like the construction: `columnName(COLUMN_KEY).equal(2)`
I doesn't sound what it should do.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407803043
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
Review Comment:
```suggestion
default ClosableCursor<T> criteriaQuery(@Nullable Transaction tx, @Nullable Criteria criteria) {
```
You use the term 'Criteria query' everywhere. So, the same name looks natural.
Otherwise, it looks confusing with smth like 'Query's criteria' = 'Criteria of a query'
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1416630009
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaVisitor.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code Visitor} defines a visitor signature for {@link Criteria} instances.
+ *
+ * @param <C> Context type.
+ */
+public interface CriteriaVisitor<C> {
Review Comment:
Moved to `internal` package into `core` module
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427876624
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQueryOptions.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.ignite.table.criteria;
+
+/**
+ * Criteria query options.
Review Comment:
it's a public API will be better to have more wide Javadoc
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427898227
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
+ RecordView<TestPojo> view = client().tables().table(TABLE_NAME).recordView(TestPojo.class);
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ private void populateData() {
+ var table = client().tables().table(TABLE_NAME).recordView();
+
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 1, COLUMN_VAL, "1")));
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 2, COLUMN_VAL, "2")));
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, 3, COLUMN_VAL, "3")));
+ }
+
+ /**
+ * Creates a matcher for matching tuple value.
+ *
+ * @param valueMatcher Matcher for matching tuple value.
+ * @return Matcher for matching tuple value.
+ */
+ private static <T> Matcher<Tuple> tupleValue(String columnName, Matcher<T> valueMatcher) {
Review Comment:
is it required?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429750714
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.ignite.sql;
+
+import java.util.Iterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * An iterator over a query results.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see KeyValueView#queryCriteria(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteria(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this iterator.
+ *
+ * @return Sequential Stream over the elements covered by this iterator.
+ */
+ default Stream<T> stream() {
Review Comment:
Deleted
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429537127
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
+ * @see KeyValueView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
Review Comment:
What do you mean? If you look at `AsyncResultSetImpl#fetchNextPage`, `ClientAsyncResultSet#fetchNextPage` both of this methods returns `this` as result and other implementations just wrap it.
If you have any ideas on how to improve javadoc, feel free to share with me, because I don't have any other ideas.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431673186
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.CursorClosedException;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.table.criteria.CriteriaQuerySource;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
Review Comment:
What useful information does this give to the user?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432624233
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
Review Comment:
Changed to `The predicate to filter entries or {@code null} to return all entries in view.`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432709150
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to run a given query. Can be {@code null} to run within implicit transaction.
Review Comment:
Removed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434204586
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistrySelfTest.java:
##########
@@ -136,7 +138,10 @@ CompletableFuture<ExecutableTable> getTable(int tableId) {
int schemaVersion = 1;
int tableVersion = 10;
- TableImpl table = new TableImpl(internalTable, schemaRegistry, new HeapLockManager(), new ConstantSchemaVersions(tableVersion));
+ TableImpl table = new TableImpl(
+ internalTable, schemaRegistry, new HeapLockManager(), new ConstantSchemaVersions(tableVersion), mock(IgniteSql.class)
+ );
Review Comment:
```suggestion
TableImpl table = new TableImpl(internalTable, schemaRegistry, new HeapLockManager(), new ConstantSchemaVersions(tableVersion),
mock(IgniteSql.class));
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434190268
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData(client(), TABLE_NAME);
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ RecordView<Tuple> view = client().tables().table(TABLE_NAME).recordView();
+
+ try (Cursor<Tuple> cur = view.queryCriteria(null, null)) {
+ assertThat(Lists.newArrayList(cur), containsInAnyOrder(
+ tupleValue(COLUMN_KEY, is(0)),
+ tupleValue(COLUMN_KEY, is(1)),
+ tupleValue(COLUMN_KEY, is(2))
+ ));
+ }
+
+ AsyncCursor<Tuple> ars = await(view.queryCriteriaAsync(null, null, builder().pageSize(2).build()));
+
+ assertNotNull(ars);
+ assertEquals(2, ars.currentPageSize());
+ await(ars.closeAsync());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
+ RecordView<TestPojo> view = client().tables().table(TABLE_NAME).recordView(TestPojo.class);
+
+ try (Cursor<TestPojo> cur = view.queryCriteria(null, null)) {
+ assertThat(Lists.newArrayList(cur), containsInAnyOrder(
+ hasProperty(COLUMN_KEY, is(0)),
+ hasProperty(COLUMN_KEY, is(1)),
+ hasProperty(COLUMN_KEY, is(2))
+ ));
+ }
+
+ AsyncCursor<TestPojo> ars = await(view.queryCriteriaAsync(null, null, builder().pageSize(2).build()));
+
+ assertNotNull(ars);
+ assertEquals(2, ars.currentPageSize());
+ await(ars.closeAsync());
+ }
+
+ private static void populateData(Ignite ignite, String tableName) {
+ RecordView<Tuple> table = ignite.tables().table(tableName).recordView();
+
+ for (int val = 0; val < 3; val++) {
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, val % 100, "val", "some string row" + val)));
+ }
+ }
+
+ /**
+ * Creates a matcher for matching tuple value.
+ *
+ * @param valueMatcher Matcher for matching tuple value.
+ * @return Matcher for matching tuple value.
+ */
+ private static <T> Matcher<Tuple> tupleValue(String columnName, Matcher<T> valueMatcher) {
Review Comment:
we have two identical matchers. Propose to extract it to separate class to have ability reuse.
For example we have org.apache.ignite.internal.testframework.matchers
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434279145
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -342,7 +347,6 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
/**
* Creates a new table manager.
*
- * @param nodeName Node name.
Review Comment:
Removed by mistake. Restored
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1424140953
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+
+/**
+ * Closeable cursor.
Review Comment:
The description does not correspond to the interface, or it needs to be expanded. At least the cursor represents the results of the SQL execution
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1407761515
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.List;
+
+/**
+ * Represents a predicate. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public interface Criteria extends Expression {
+ /**
+ * Create a predicate for testing the column is equal to a given value.
+ *
+ * @param columnName Column name.
+ * @param value Column value.
+ * @return the created <b>equal</b> predicate instance.
+ */
+ static <T> Criteria equal(String columnName, T value) {
+ return Operation.create("{0} = {1}", List.of(new StaticText(columnName), new Constant<>(value)));
+ }
+
+ /**
+ * Create a predicate for testing the column is equal to a given value.
Review Comment:
Looks like copy-paste from above, what does `not` do?
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaVisitor.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code Visitor} defines a visitor signature for {@link Criteria} instances.
+ *
+ * @param <C> Context type.
+ */
+public interface CriteriaVisitor<C> {
Review Comment:
Do we need this in public API?
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.List;
+
+/**
+ * Represents a predicate. Implementations of this interface are basic building blocks for performing criteria queries.
+ */
+public interface Criteria extends Expression {
+ /**
+ * Create a predicate for testing the column is equal to a given value.
Review Comment:
```suggestion
* Creates a predicate that tests whether the column value is equal to the given value.
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427860953
##########
modules/api/src/main/java/org/apache/ignite/sql/ClosableCursor.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.ignite.sql;
+
+import java.util.Iterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * An iterator over a query results.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see KeyValueView#queryCriteria(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteria(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface ClosableCursor<T> extends Iterator<T>, AutoCloseable {
+ /**
+ * Returns a sequential Stream over the elements covered by this iterator.
+ *
+ * @return Sequential Stream over the elements covered by this iterator.
+ */
+ default Stream<T> stream() {
Review Comment:
Why do you want the utility method has as part of interface?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427883352
##########
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java:
##########
@@ -380,4 +388,34 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable
return ClientDataStreamer.streamData(publisher, opts, batchSender, provider, tbl);
}
+
+ private CompletableFuture<AsyncResultSet<Tuple>> executeAsync(
+ @Nullable Transaction tx,
+ @Nullable Criteria criteria,
+ CriteriaQueryOptions opts
+ ) {
+ //TODO: implement serialization of criteria to SQL https://issues.apache.org/jira/browse/IGNITE-20879
+ var query = "SELECT * FROM " + tbl.name();
+
+ var statement = tbl.sql().statementBuilder().query(query).pageSize(opts.pageSize()).build();
Review Comment:
`var` is not allowed in the context
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1427897389
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static java.util.stream.Collectors.toList;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData();
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ var view = client().tables().table(TABLE_NAME).recordView();
+
+ var res = view.queryCriteria(null, null).stream().collect(toList());
+ assertThat(res, hasSize(3));
+
+ var ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
+ assertEquals(2, ars.currentPageSize());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
Review Comment:
I just see check sizes, but not content. Why?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1416630525
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/StaticText.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.ignite.table.criteria;
+
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * {@code StaticText} represents a static text element.
+ */
+public class StaticText implements Expression {
Review Comment:
Renamed `Constant` to `Argument`. Moved this changes to #2933
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1431377915
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents criteria query facade which can be used to iterate through table entries filtered by some predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
+ * @param criteria Will accept all the entries if {@code null}.
+ * @return Iterator for table entries filtered by some predicate.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Filter table entries by some predicate.
+ *
+ * @param tx Transaction to filter table entries within or {@code null} to run within implicit transaction.
Review Comment:
It seems a transaction is not required to filter the table, but the queries are executed in a transaction
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434145652
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
I would say `Async` specify porpose of the adapter, but it should looks more right like `CursorSyncAdaptar`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434279980
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData(client(), TABLE_NAME);
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ RecordView<Tuple> view = client().tables().table(TABLE_NAME).recordView();
+
+ try (Cursor<Tuple> cur = view.queryCriteria(null, null)) {
+ assertThat(Lists.newArrayList(cur), containsInAnyOrder(
+ tupleValue(COLUMN_KEY, is(0)),
+ tupleValue(COLUMN_KEY, is(1)),
+ tupleValue(COLUMN_KEY, is(2))
+ ));
+ }
+
+ AsyncCursor<Tuple> ars = await(view.queryCriteriaAsync(null, null, builder().pageSize(2).build()));
+
+ assertNotNull(ars);
+ assertEquals(2, ars.currentPageSize());
+ await(ars.closeAsync());
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordPojoView() {
+ RecordView<TestPojo> view = client().tables().table(TABLE_NAME).recordView(TestPojo.class);
+
+ try (Cursor<TestPojo> cur = view.queryCriteria(null, null)) {
+ assertThat(Lists.newArrayList(cur), containsInAnyOrder(
+ hasProperty(COLUMN_KEY, is(0)),
+ hasProperty(COLUMN_KEY, is(1)),
+ hasProperty(COLUMN_KEY, is(2))
+ ));
+ }
+
+ AsyncCursor<TestPojo> ars = await(view.queryCriteriaAsync(null, null, builder().pageSize(2).build()));
+
+ assertNotNull(ars);
+ assertEquals(2, ars.currentPageSize());
+ await(ars.closeAsync());
+ }
+
+ private static void populateData(Ignite ignite, String tableName) {
+ RecordView<Tuple> table = ignite.tables().table(tableName).recordView();
+
+ for (int val = 0; val < 3; val++) {
+ table.insert(null, Tuple.create(Map.of(COLUMN_KEY, val % 100, "val", "some string row" + val)));
+ }
+ }
+
+ /**
+ * Creates a matcher for matching tuple value.
+ *
+ * @param valueMatcher Matcher for matching tuple value.
+ * @return Matcher for matching tuple value.
+ */
+ private static <T> Matcher<Tuple> tupleValue(String columnName, Matcher<T> valueMatcher) {
Review Comment:
Created `TupleMatcher`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432704288
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents an object which can be queried with criteria.
+ *
+ * @param <T> Entry type.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @throws SqlException If failed.
+ */
+ default ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteria(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Criteria query over cache entries.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @param opts Criteria query options.
+ * @throws SqlException If failed.
+ */
+ ClosableCursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts);
+
+ /**
+ * Execute criteria query over cache entries in an asynchronous way.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
+ * @param criteria If {@code null} then all entries will be returned.
+ * @return Operation future.
+ * @throws SqlException If failed.
+ */
+ default CompletableFuture<? extends AsyncClosableCursor<T>> queryCriteriaAsync(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return queryCriteriaAsync(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Execute criteria query over cache entries in an asynchronous way.
+ *
+ * @param tx Transaction to execute the query within or {@code null}.
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1432775038
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData(client(), TABLE_NAME);
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ RecordView<Tuple> view = client().tables().table(TABLE_NAME).recordView();
+
+ List<Tuple> res = Lists.newArrayList(view.queryCriteria(null, null));
+ assertThat(res, containsInAnyOrder(
+ tupleValue(COLUMN_KEY, is(0)),
+ tupleValue(COLUMN_KEY, is(1)),
+ tupleValue(COLUMN_KEY, is(2))
+ ));
+
+ AsyncClosableCursor<Tuple> ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
Review Comment:
it would be nice to close cursor at the end each 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1433744287
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientCriteriaQueryTest.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.ignite.internal.runner.app.client;
+
+import static org.apache.ignite.table.criteria.CriteriaQueryOptions.builder;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.google.common.collect.Lists;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.sql.async.AsyncClosableCursor;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+
+/**
+ * Thin client criteria query test.
+ */
+@SuppressWarnings("resource")
+public class ItThinClientCriteriaQueryTest extends ItAbstractThinClientTest {
+ /** {@inheritDoc} */
+ @Override
+ @BeforeAll
+ void beforeAll(TestInfo testInfo, @WorkDirectory Path workDir) throws InterruptedException {
+ super.beforeAll(testInfo, workDir);
+
+ populateData(client(), TABLE_NAME);
+ }
+
+ @Test
+ public void testBasicQueryCriteriaRecordBinaryView() {
+ RecordView<Tuple> view = client().tables().table(TABLE_NAME).recordView();
+
+ List<Tuple> res = Lists.newArrayList(view.queryCriteria(null, null));
+ assertThat(res, containsInAnyOrder(
+ tupleValue(COLUMN_KEY, is(0)),
+ tupleValue(COLUMN_KEY, is(1)),
+ tupleValue(COLUMN_KEY, is(2))
+ ));
+
+ AsyncClosableCursor<Tuple> ars = view.queryCriteriaAsync(null, null, builder().pageSize(2).build()).join();
Review Comment:
Added close cursor.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "agura (via GitHub)" <gi...@apache.org>.
agura commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434021717
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
Review Comment:
> to executes a query to a view
What kind of view do you mean? I believe it is record view. Please, use right terms.
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
Review Comment:
I don't like the naming. You don't query criteria, instead you execute query which is based on criteria. In addition I see a lot of noise with criteria, e.g. query**Criteria**(null, **criteria**, **criteria**Opts).
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in view.
Review Comment:
> to return all entries in view.
What kind of view do you mean? I believe it is record view. Please, use right terms. The view is a standard term for SQL. In order to have right semantic please use record view term here and everywhere.
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SyncCursorAdapter.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.table.criteria;
+
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.convertToPublicFuture;
+import static org.apache.ignite.internal.lang.IgniteExceptionMapperUtil.mapToPublicException;
+import static org.apache.ignite.internal.util.ExceptionUtils.sneakyThrow;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.Iterator;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+
+/**
+ * Synchronous wrapper over {@link AsyncCursor}.
+ */
+public class SyncCursorAdapter<T> implements Cursor<T> {
Review Comment:
If it is not Async it is obviously Sync. Just redundant Sync word.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1434976896
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -381,7 +386,8 @@ public TableManager(
SchemaSyncService schemaSyncService,
CatalogService catalogService,
HybridTimestampTracker observableTimestampTracker,
- PlacementDriver placementDriver
+ PlacementDriver placementDriver,
+ Supplier<IgniteSql> sql
Review Comment:
Unfortunately with current approach it can't be fixed. I perhaps we rethinking the approach and remove SQL here in the future
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "agura (via GitHub)" <gi...@apache.org>.
agura commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1436056239
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> queryCriteria(@Nullable Transaction tx, @Nullable Criteria criteria) {
Review Comment:
@nva I didn't suggest any specific name. Just pointed out a possible problem. May be `query` is okay.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1440566449
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/CriteriaQuerySource.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.ignite.table.criteria;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.lang.AsyncCursor;
+import org.apache.ignite.lang.Cursor;
+import org.apache.ignite.sql.SqlException;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a criteria query facade that can be used to executes a query to a record view using a predicate.
+ *
+ * @param <T> The type of elements returned by iterator.
+ */
+public interface CriteriaQuerySource<T> {
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ default Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria) {
+ return query(tx, criteria, CriteriaQueryOptions.DEFAULT);
+ }
+
+ /**
+ * Executes a query to a record view using a predicate.
+ *
+ * @param tx Transaction to execute the query within or {@code null} to run within implicit transaction.
+ * @param criteria The predicate to filter entries or {@code null} to return all entries in record view.
+ * @param opts Criteria query options.
+ * @return Iterator with query results.
+ * @throws SqlException If failed.
+ */
+ Cursor<T> query(@Nullable Transaction tx, @Nullable Criteria criteria, CriteriaQueryOptions opts);
Review Comment:
Allowed to pass null value for options
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20878 Basic criteria queries for record view. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #2882:
URL: https://github.com/apache/ignite-3/pull/2882#discussion_r1429537127
##########
modules/api/src/main/java/org/apache/ignite/sql/async/AsyncClosableCursor.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.ignite.sql.async;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.sql.ClosableCursor;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.table.KeyValueView;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.criteria.Criteria;
+import org.apache.ignite.table.criteria.CriteriaQueryOptions;
+import org.apache.ignite.tx.Transaction;
+
+/**
+ * Provides methods for iterate over query results in an asynchronous way.
+ *
+ * @param <T> The type of elements returned by this iterator.
+ *
+ * @see ClosableCursor
+ * @see KeyValueView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ * @see RecordView#queryCriteriaAsync(Transaction, Criteria, CriteriaQueryOptions)
+ */
+public interface AsyncClosableCursor<T> {
+ /**
+ * Returns the current page content if the query returns rows.
+ *
+ * @return Iterable set of rows.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ Iterable<T> currentPage();
+
+ /**
+ * Returns the current page size if the query return rows.
+ *
+ * @return The size of {@link #currentPage()}.
+ * @throws NoRowSetExpectedException if no row set is returned.
+ */
+ int currentPageSize();
+
+ /**
+ * Fetches the next page of results asynchronously.
+ * The current page is changed after the future completion.
+ * The methods {@link #currentPage()}, {@link #currentPageSize()}, {@link #hasMorePages()}
+ * use the current page and return consistent results between complete last page future and call {@code fetchNextPage}.
+ *
+ * @return A future which will be completed when next page will be fetched and set as the current page.
+ * The future will return {@code this} for chaining.
Review Comment:
What do you mean? If you look at `AsyncResultSetImpl#fetchNextPage`, `ClientAsyncResultSet#fetchNextPage` both of this methods returns `this` as result and other implantations just wrap it.
If you have any ideas on how to improve javadoc, feel free to share with me, because I don't have any other ideas.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org