[PR] IGNITE-22075 GC doesn't wait for RO transactions [ignite-3]
tkalkirill opened a new pull request, #3650: URL: https://github.com/apache/ignite-3/pull/3650 https://issues.apache.org/jira/browse/IGNITE-22075 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- Change summary where - 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] IBinarizable : Mapping complex objects [ignite]
ptupitsyn commented on issue #11325: URL: https://github.com/apache/ignite/issues/11325#issuecomment-2071369006 If you use `GetRawWriter`, it will work with key/value API, but not with SQL. > the attributes of Address type is not getting saved to Employee table Please share the code. What do you mean by "not getting saved to Employee table" - how do you see that? -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] I am running a 3-node external cluster, but I find that the memory is growing slowly. By checking the top, I can see that the buffer/cache has been growing slowly and has not been released. What
1ssqq1lxr opened a new issue, #11326: URL: https://github.com/apache/ignite/issues/11326 ![ca10438c80c29f2080bf9f99d813531](https://github.com/apache/ignite/assets/19258331/a41f87df-a64e-443d-b90d-c3d8bad63f2e) -- 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-unsubscr...@ignite.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Continuous query using NodeJS client [ignite]
satishviswanathan commented on issue #11312: URL: https://github.com/apache/ignite/issues/11312#issuecomment-2071062070 @sk0x50 thank you for your response. That was helpful -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591: URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1575066409 ## modules/runner/src/testFixtures/java/org/apache/ignite/internal/table/NodeUtils.java: ## @@ -49,16 +50,16 @@ public class NodeUtils { * * @param nodes Nodes collection. * @param groupId Group id. - * @param preferablePrimary Primary replica name which is preferred for being primary or {@code null}. + * @param preferablePrimaryFilter Primary replica preferable nodes filter, accepts the node consistent id. * @return New primary replica name. * @throws InterruptedException If failed. */ public static String transferPrimary( Collection nodes, ReplicationGroupId groupId, -@Nullable String preferablePrimary +@Nullable Predicate preferablePrimaryFilter Review Comment: Is it all about ::contains support? Don't you think that it's a bit too complicated? Anyway, I'd definitely preserve original method as sugar. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591: URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1574736503 ## modules/runner/src/testFixtures/java/org/apache/ignite/internal/table/NodeUtils.java: ## @@ -49,16 +50,16 @@ public class NodeUtils { * * @param nodes Nodes collection. * @param groupId Group id. - * @param preferablePrimary Primary replica name which is preferred for being primary or {@code null}. + * @param preferablePrimaryFilter Primary replica preferable nodes filter, accepts the node consistent id. * @return New primary replica name. * @throws InterruptedException If failed. */ public static String transferPrimary( Collection nodes, ReplicationGroupId groupId, -@Nullable String preferablePrimary +@Nullable Predicate preferablePrimaryFilter Review Comment: Should we move it towards node id instead of node name in order to support transferring primary to the same node after restart? ## modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTransactionTestUtils.java: ## @@ -0,0 +1,204 @@ +/* + * 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 java.util.Objects.requireNonNull; +import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.stream.Collectors.toSet; +import static org.apache.ignite.internal.TestWrappers.unwrapIgniteTransaction; +import static org.apache.ignite.internal.TestWrappers.unwrapRecordBinaryViewImpl; +import static org.apache.ignite.internal.TestWrappers.unwrapTableImpl; +import static org.apache.ignite.internal.distributionzones.rebalance.RebalanceUtil.stablePartAssignmentsKey; +import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; +import org.apache.ignite.internal.affinity.Assignment; +import org.apache.ignite.internal.affinity.Assignments; +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.lang.ByteArray; +import org.apache.ignite.internal.metastorage.Entry; +import org.apache.ignite.internal.metastorage.MetaStorageManager; +import org.apache.ignite.internal.placementdriver.ReplicaMeta; +import org.apache.ignite.internal.replicator.ReplicationGroupId; +import org.apache.ignite.internal.replicator.TablePartitionId; +import org.apache.ignite.internal.schema.BinaryRowEx; +import org.apache.ignite.internal.tx.impl.ReadWriteTransactionImpl; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.jetbrains.annotations.Nullable; + +/** + * Test utils for transaction integration tests. + */ +public class ItTransactionTestUtils { +/** + * Get the names of the nodes that are assignments of the given partition. + * + * @param node Any node in the cluster. + * @param grpId Group id. + * @return Node names. + */ +public static Set partitionAssignment(IgniteImpl node, TablePartitionId grpId) { +MetaStorageManager metaStorageManager = node.metaStorageManager(); + +ByteArray stableAssignmentKey = stablePartAssignmentsKey(grpId); + +CompletableFuture assignmentEntryFut = metaStorageManager.get(stableAssignmentKey); + +assertThat(assignmentEntryFut, willCompleteSuccessfully()); + +Entry e = assignmentEntryFut.join(); + +assertNotNull(e); +assertFalse(e.empty()); +assertFalse(e.tombstone()); + +Set a = requireNonNull(Assignments.fromBytes(e.value())).nodes(); + +return a.stream().filter(Assignment::isPeer).map(Assignment::consistentId).collect(toSet()); +} + +/** + * Calculate the partition id on which the given tuple would be placed. + * + * @param node Any node in the cluster. + * @param tableName Table name. + *
Re: [PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.0 [ignite-3]
dependabot[bot] closed pull request #3483: Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.0 URL: https://github.com/apache/ignite-3/pull/3483 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.0 [ignite-3]
dependabot[bot] commented on PR #3483: URL: https://github.com/apache/ignite-3/pull/3483#issuecomment-2069916710 Superseded by #3649. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.1 [ignite-3]
dependabot[bot] opened a new pull request, #3649: URL: https://github.com/apache/ignite-3/pull/3649 Bumps [org.rocksdb:rocksdbjni](https://github.com/facebook/rocksdb) from 8.11.3 to 9.0.1. Release notes Sourced from https://github.com/facebook/rocksdb/releases;>org.rocksdb:rocksdbjni's releases. RocksDB 9.0.1 9.0.1 (2024-04-11) Bug Fixes Fixed CMake Javadoc and source jar builds Fixed Java SstFileMetaData to prevent throwing java.lang.NoSuchMethodError RocksDB 9.0.0 9.0.0 (2024-02-16) New Features Provide support for FSBuffer for point lookups. Also added support for scans and compactions that don't go through prefetching. *Make SstFileWriter create SST files without persisting user defined timestamps when the Option.persist_user_defined_timestamps flag is set to false. Add support for user-defined timestamps in APIs DeleteFilesInRanges and GetPropertiesOfTablesInRange. Mark wal_compression feature as production-ready. Currently only compatible with ZSTD compression. Public API Changes Allow setting Stderr logger via C API Declare one Get and one MultiGet variant as pure virtual, and make all the other variants non-overridable. The methods required to be implemented by derived classes of DB allow returning timestamps. It is up to the implementation to check and return an error if timestamps are not supported. The non-batched MultiGet APIs are reimplemented in terms of batched MultiGet, so callers might see a performance improvement. Exposed mode option to Rate Limiter via c api. Removed deprecated option access_hint_on_compaction_start Removed deprecated option ColumnFamilyOptions::check_flush_compaction_key_order *Remove the default WritableFile::GetFileSize and FSWritableFile::GetFileSize implementation that returns 0 and make it pure virtual, so that subclasses are enforced to explicitly provide an implementation. Removed deprecated option ColumnFamilyOptions::level_compaction_dynamic_file_size *Removed tickers with typos rocksdb.error.handler.bg.errro.count, rocksdb.error.handler.bg.io.errro.count, rocksdb.error.handler.bg.retryable.io.errro.count. Remove the force mode for EnableFileDeletions API because it is unsafe with no known legitimate use. Removed deprecated option ColumnFamilyOptions::ignore_max_compaction_bytes_for_input sst_dump --command=check now compares the number of records in a table with num_entries in table property, and reports corruption if there is a mismatch. API SstFileDumper::ReadSequential() is updated to optionally do this verification. (https://redirect.github.com/facebook/rocksdb/issues/12322;>#12322) Behavior Changes format_version=6 is the new default setting in BlockBasedTableOptions, for more robust data integrity checking. DBs and SST files written with this setting cannot be read by RocksDB versions before 8.6.0. Compactions can be scheduled in parallel in an additional scenario: multiple files are marked for compaction within a single column family For leveled compaction, RocksDB will try to do intra-L0 compaction if the total L0 size is small compared to Lbase (https://redirect.github.com/facebook/rocksdb/issues/12214;>#12214). Users with atomic_flush=true are more likely to see the impact of this change. Bug Fixes Fixed a data race in DBImpl::RenameTempFileToOptionsFile. Fix some perf context statistics error in write steps. which include missing write_memtable_time in unordered_write. missing write_memtable_time in PipelineWrite when Writer stat is STATE_PARALLEL_MEMTABLE_WRITER. missing write_delay_time when calling DelayWrite in WriteImplWALOnly function. Fixed a bug that can, under rare circumstances, cause MultiGet to return an incorrect result for a duplicate key in a MultiGet batch. Fix a bug where older data of an ingested key can be returned for read when universal compaction is used RocksDB 8.11.4 8.11.4 (2024-04-09) Bug Fixes Fixed CMake Javadoc build Fixed Java SstFileMetaData to prevent throwing java.lang.NoSuchMethodError Changelog Sourced from https://github.com/facebook/rocksdb/blob/v9.0.1/HISTORY.md;>org.rocksdb:rocksdbjni's changelog. 9.0.1 (04/11/2024) Bug Fixes Fixed CMake Javadoc and source jar builds Fixed Java SstFileMetaData to prevent throwing java.lang.NoSuchMethodError 9.0.0 (02/16/2024) New Features Provide support for FSBuffer for point lookups. Also added support for scans and compactions that don't go through prefetching. *Make SstFileWriter create SST files without persisting user defined timestamps when the Option.persist_user_defined_timestamps flag is set to false. Add support for user-defined timestamps in APIs DeleteFilesInRanges and GetPropertiesOfTablesInRange. Mark wal_compression feature as production-ready. Currently only
[PR] Bump fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3 [ignite-3]
dependabot[bot] opened a new pull request, #3648: URL: https://github.com/apache/ignite-3/pull/3648 Bumps fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3. [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=fr.inria.gforge.spoon:spoon-core=gradle=10.4.3-beta-20=11.0.1-beta-3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-22081 Delete TransactionSerializationException and TransactionAlreadyCompletedException [ignite]
lordgarrish opened a new pull request, #11324: URL: https://github.com/apache/ignite/pull/11324 Thank you for submitting the pull request to the Apache Ignite. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### The Contribution Checklist - [ ] 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 pull request body describes changes that have been made. The description explains _WHAT_ and _WHY_ was made instead of _HOW_. - [ ] The pull request title is treated as the final commit message. The following pattern must be used: `IGNITE- Change summary` where `` - number of JIRA issue. - [ ] A reviewer has been mentioned through the JIRA comments (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) - [ ] The pull request has been checked by the Teamcity Bot and the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html)) ### Notes - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute) - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules) - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines) - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot) If you need any help, please email d...@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
xtern merged PR #3626: URL: https://github.com/apache/ignite-3/pull/3626 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-21850 [ignite-3]
PakhomovAlexander opened a new pull request, #3646: URL: https://github.com/apache/ignite-3/pull/3646 (no 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21888 Remove MvccVersion and MvccUpdateVersionAware [ignite]
anton-vinogradov merged PR #11320: URL: https://github.com/apache/ignite/pull/11320 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21939 Sql. Cover SQL F302-02(INTERSECT [ALL] table operator) feature by tests. [ignite-3]
xtern merged PR #3636: URL: https://github.com/apache/ignite-3/pull/3636 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21940 Sql. Cover SQL F304(EXCEPT ALL table operator) feature by tests [ignite-3]
xtern merged PR #3635: URL: https://github.com/apache/ignite-3/pull/3635 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-19949 Fix dynamic completer for same options [ignite-3]
PakhomovAlexander merged PR #3644: URL: https://github.com/apache/ignite-3/pull/3644 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22083 Invalidate fragment mapping cache when logical topology changes [ignite-3]
xtern merged PR #3637: URL: https://github.com/apache/ignite-3/pull/3637 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
xtern commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574704985 ## modules/api/src/main/java/org/apache/ignite/sql/Statement.java: ## @@ -125,13 +105,6 @@ interface StatementBuilder { */ StatementBuilder defaultSchema(String schema); -/** - * Returns a page size - the maximum number of result rows that can be fetched at a time. - * - * @return Maximum number of rows per page. - */ -int pageSize(); - /** * Sets a page size - the maximum number of result rows that can be fetched at a time. Review Comment: thanks, javadoc extended. ## modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientMetricsTest.java: ## @@ -119,10 +118,10 @@ public void testErroneousFlow() throws Exception { } private void assertMetricValue(MetricSet metricSet, String metricName, Object expectedValue) throws InterruptedException { -assertTrue( -waitForCondition( -() -> expectedValue.toString().equals(metricSet.get(metricName).getValueAsString()), -1000) -); +waitForCondition( +() -> expectedValue.toString().equals(metricSet.get(metricName).getValueAsString()), +1000); + +assertEquals(expectedValue.toString(), metricSet.get(metricName).getValueAsString()); Review Comment: done, thanks -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21720 Sql. Implement hash join [ignite-3]
korlov42 commented on code in PR #3608: URL: https://github.com/apache/ignite-3/pull/3608#discussion_r1574679534 ## modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItJoinTest.java: ## @@ -843,6 +1067,7 @@ private static Stream joinTypes() { Stream types = Arrays.stream(JoinType.values()) // TODO: https://issues.apache.org/jira/browse/IGNITE-21286 remove filter below .filter(type -> type != JoinType.CORRELATED) +.filter(type -> type != JoinType.HASHJOIN) Review Comment: let's add TODO referencing a ticket with support of IS NOT DISTINCT operator ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/HashJoinNode.java: ## @@ -0,0 +1,623 @@ +/* + * 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.sql.engine.exec.rel; + +import static org.apache.ignite.internal.sql.engine.util.TypeUtils.rowSchemaFromRelTypes; + +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.rel.core.JoinInfo; +import org.apache.calcite.rel.core.JoinRelType; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.ignite.internal.sql.engine.exec.ExecutionContext; +import org.apache.ignite.internal.sql.engine.exec.RowHandler; +import org.apache.ignite.internal.sql.engine.exec.row.RowSchema; +import org.jetbrains.annotations.Nullable; + +/** HashJoin implementor. */ +public abstract class HashJoinNode extends AbstractRightMaterializedJoinNode { +Map hashStore = new Object2ObjectOpenHashMap<>(); +protected final RowHandler handler; + +final Collection leftJoinPositions; +private final Collection rightJoinPositions; + +final boolean touchResults; + +Iterator rightIt = Collections.emptyIterator(); + +private HashJoinNode(ExecutionContext ctx, JoinInfo joinInfo, boolean touch) { +super(ctx); + +handler = ctx.rowHandler(); +touchResults = touch; + +leftJoinPositions = joinInfo.leftKeys.toIntegerList(); +rightJoinPositions = joinInfo.rightKeys.toIntegerList(); +} + +@Override +protected void rewindInternal() { +rightIt = Collections.emptyIterator(); + +hashStore.clear(); + +super.rewindInternal(); +} + +/** Supplied algorithm implementation. */ +public static HashJoinNode create(ExecutionContext ctx, RelDataType outputRowType, +RelDataType leftRowType, RelDataType rightRowType, JoinRelType joinType, JoinInfo joinInfo) { + +switch (joinType) { +case INNER: +return new InnerHashJoin<>(ctx, joinInfo); + +case LEFT: { +RowSchema rightRowSchema = rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType)); +RowHandler.RowFactory rightRowFactory = ctx.rowHandler().factory(rightRowSchema); + +return new LeftHashJoin<>(ctx, rightRowFactory, joinInfo); +} + +case RIGHT: { +RowSchema leftRowSchema = rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType)); +RowHandler.RowFactory leftRowFactory = ctx.rowHandler().factory(leftRowSchema); + +return new RightHashJoin<>(ctx, leftRowFactory, joinInfo); +} + +case FULL: { +RowSchema leftRowSchema = rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType)); +RowSchema rightRowSchema = rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType)); +RowHandler.RowFactory leftRowFactory = ctx.rowHandler().factory(leftRowSchema); +RowHandler.RowFactory rightRowFactory = ctx.rowHandler().factory(rightRowSchema); + +return new FullOuterHashJoin<>(ctx, leftRowFactory, rightRowFactory, joinInfo); +} + +case SEMI: +return new SemiHashJoin<>(ctx, joinInfo); +
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
xtern commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574688833 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: > Maybe It is better to make it impossible to create invalid statement Thanks, it's impossible now. Validation is done in `StatementImpl`. I just left assertion that pageSize is positive here. ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: > Maybe It is better to make it impossible to create invalid statement Thanks, it's impossible now. Validation is done in `StatementImpl`. I just left an assertion that pageSize is positive here. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
xtern commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574688833 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: > Maybe It is better to make it impossible to create invalid statement Thanks, it's impossible now. Validation is done in `StatementImpl`. I just kept assertion that pageSize is positive here. ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: > Maybe It is better to make it impossible to create invalid statement Thanks, it's impossible now. Validation is done in `StatementImpl`. I just kept assertion that pageSize is positive here. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-21908 Add metrics of distribution among stripes in disruptor [ignite-3]
vldpyatkov opened a new pull request, #3645: URL: https://github.com/apache/ignite-3/pull/3645 https://issues.apache.org/jira/browse/IGNITE-21908 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21835 Cleanup MvccUtils and enum RowData [ignite]
shishkovilja commented on PR #11318: URL: https://github.com/apache/ignite/pull/11318#issuecomment-2069218679 @J-Bakuli , let's close this pull request. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-21345 TxState and MvccSnapshot removal [ignite]
shishkovilja opened a new pull request, #11323: URL: https://github.com/apache/ignite/pull/11323 Thank you for submitting the pull request to the Apache Ignite. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### The Contribution Checklist - [ ] 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 pull request body describes changes that have been made. The description explains _WHAT_ and _WHY_ was made instead of _HOW_. - [ ] The pull request title is treated as the final commit message. The following pattern must be used: `IGNITE- Change summary` where `` - number of JIRA issue. - [ ] A reviewer has been mentioned through the JIRA comments (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) - [ ] The pull request has been checked by the Teamcity Bot and the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html)) ### Notes - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute) - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules) - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines) - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot) If you need any help, please email d...@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Apache Ignite 2.15.0 startup failure using OpenJDK 17 version [ignite]
sk0x50 closed issue #10747: Apache Ignite 2.15.0 startup failure using OpenJDK 17 version URL: https://github.com/apache/ignite/issues/10747 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
AMashenkov commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574534112 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: I guess no, because Statement is an interface. Maybe, we can move all assertions to a validate method and throw validation exception instead? We could have these checks in StatementBuilder, and just skip validation here for statements of our internal class. But anyway, we need validation for any other 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22069 Optimize entries expiration [ignite]
yurinaryshkin commented on code in PR #11319: URL: https://github.com/apache/ignite/pull/11319#discussion_r1574534314 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheOffheapManager.java: ## @@ -1108,7 +1110,28 @@ private Metas getOrAllocateCacheMetas() throws IgniteCheckedException { try { int cleared = 0; +// Use random shift to reduce contention. +int shift = ThreadLocalRandom.current().nextInt(F.size(cacheDataStores().iterator())); + +int cnt = 0; +for (CacheDataStore store : cacheDataStores()) { Review Comment: As far as I understood, the logic here is to split cacheDataStores() into 2 parts at random index and after that process right sublist before the left sublist. Can this change be simplified to minimize code duplication? For example, construct a new list from 2 sublists and process it as before. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
AMashenkov commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574534112 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: I guess no, because Statement is an interface. Maybe, we can move all assertions to a validate method and throw validation exception 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21988 Don't allow reading from index storage if it is in build state [ignite-3]
tkalkirill merged PR #3561: URL: https://github.com/apache/ignite-3/pull/3561 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-19949 Fix dynamic completer for same options [ignite-3]
PakhomovAlexander opened a new pull request, #3644: URL: https://github.com/apache/ignite-3/pull/3644 Now you can define --option for multiple commands, and they won't be conflicting. https://issues.apache.org/jira/browse/IGNITE-19949 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21943: Cover SQL F561(Full value expressions) feature by tests [ignite-3]
ygerzhedovich commented on code in PR #3641: URL: https://github.com/apache/ignite-3/pull/3641#discussion_r1574454489 ## modules/sql-engine/src/integrationTest/sql/types/row/test_row.test: ## @@ -0,0 +1,45 @@ +# name: sql/types/row/test_row.test +# description: SQL feature F561 Full value expressions +# feature: F561 +# group: [row] + +statement ok +CREATE TABLE emp (empid INTEGER PRIMARY KEY, empname VARCHAR, empage INTEGER, salary INTEGER) + +statement ok +INSERT INTO emp VALUES(1, 'Johnah', 47, 1200) + +statement ok +INSERT INTO emp VALUES(2, 'Maria', 47, 2000) + +statement ok +INSERT INTO emp VALUES(3, 'Noah', 47, 1200) + +statement ok +INSERT INTO emp VALUES(4, 'Dave', 42, 1700) + +query T rowsort Review Comment: what reason to have `rowsort` option here and below? -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22069 Optimize entries expiration [ignite]
alex-plekhanov commented on code in PR #11319: URL: https://github.com/apache/ignite/pull/11319#discussion_r1574406245 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/tree/BPlusTree.java: ## @@ -2169,6 +2169,20 @@ public List remove(L lower, L upper, int limit) throws IgniteCheckedException return Collections.unmodifiableList(rmvOp.removedRows); } +/** + * @param lower Lower bound (inclusive). + * @param upper Upper bound (inclusive). + * @param x Implementation specific argument. + * @param limit Limit of processed entries by single call, {@code 0} for no limit. Review Comment: 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22069 Optimize entries expiration [ignite]
alex-plekhanov commented on code in PR #11319: URL: https://github.com/apache/ignite/pull/11319#discussion_r1574405268 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheOffheapManager.java: ## @@ -2761,43 +2774,37 @@ private int purgeExpiredInternal( long now = U.currentTimeMillis(); -GridCursor cur; - -if (grp.sharedGroup()) -cur = pendingTree.find(new PendingRow(cctx.cacheId()), new PendingRow(cctx.cacheId(), now, 0)); -else -cur = pendingTree.find(null, new PendingRow(CU.UNDEFINED_CACHE_ID, now, 0)); - -if (!cur.next()) -return 0; - -GridCacheVersion obsoleteVer = null; +int cacheId = grp.sharedGroup() ? cctx.cacheId() : CU.UNDEFINED_CACHE_ID; int cleared = 0; do { -PendingRow row = cur.get(); +List rows = pendingTree.remove(new PendingRow(cacheId, Long.MIN_VALUE, 0), +new PendingRow(cacheId, now, 0), amount - cleared); + +if (rows.isEmpty()) +break; -if (amount != -1 && cleared > amount) -return cleared; +for (PendingRow row : rows) { +row.key.partition(partId); -assert row.key != null && row.link != 0 && row.expireTime != 0 : row; +assert row.key != null && row.link != 0 && row.expireTime != 0 : row; -row.key.partition(partId); +GridCacheVersion obsoleteVer = null; -if (pendingTree.removex(row)) { if (obsoleteVer == null) obsoleteVer = cctx.cache().nextVersion(); -GridCacheEntryEx e1 = cctx.cache().entryEx(row.key); +GridCacheEntryEx entry = cctx.cache().entryEx(row.key instanceof KeyCacheObjectImpl +? new ExpiredKeyCacheObject((KeyCacheObjectImpl)row.key, row.expireTime, row.link) : row.key); -if (e1 != null) -c.apply(e1, obsoleteVer); +if (entry != null) +c.apply(entry, obsoleteVer); } -cleared++; +cleared += rows.size(); } -while (cur.next()); +while (amount < 0 || cleared < amount); Review Comment: See javadoc for `IgniteCacheOffheapManager#expire`, -1 as parameter assume no limit. We never use it in code, but it was supported in the previous implementation and it's easy to support now, so I decided to keep this condition. -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22069 Optimize entries expiration [ignite]
yurinaryshkin commented on code in PR #11319: URL: https://github.com/apache/ignite/pull/11319#discussion_r1574357146 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/tree/BPlusTree.java: ## @@ -2169,6 +2169,20 @@ public List remove(L lower, L upper, int limit) throws IgniteCheckedException return Collections.unmodifiableList(rmvOp.removedRows); } +/** + * @param lower Lower bound (inclusive). + * @param upper Upper bound (inclusive). + * @param x Implementation specific argument. + * @param limit Limit of processed entries by single call, {@code 0} for no limit. Review Comment: Both invocations of the method pass -1 as parameter limit. Should this value be added to the 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22069 Optimize entries expiration [ignite]
yurinaryshkin commented on code in PR #11319: URL: https://github.com/apache/ignite/pull/11319#discussion_r1574328812 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheOffheapManager.java: ## @@ -2761,43 +2774,37 @@ private int purgeExpiredInternal( long now = U.currentTimeMillis(); -GridCursor cur; - -if (grp.sharedGroup()) -cur = pendingTree.find(new PendingRow(cctx.cacheId()), new PendingRow(cctx.cacheId(), now, 0)); -else -cur = pendingTree.find(null, new PendingRow(CU.UNDEFINED_CACHE_ID, now, 0)); - -if (!cur.next()) -return 0; - -GridCacheVersion obsoleteVer = null; +int cacheId = grp.sharedGroup() ? cctx.cacheId() : CU.UNDEFINED_CACHE_ID; int cleared = 0; do { -PendingRow row = cur.get(); +List rows = pendingTree.remove(new PendingRow(cacheId, Long.MIN_VALUE, 0), +new PendingRow(cacheId, now, 0), amount - cleared); + +if (rows.isEmpty()) +break; -if (amount != -1 && cleared > amount) -return cleared; +for (PendingRow row : rows) { +row.key.partition(partId); -assert row.key != null && row.link != 0 && row.expireTime != 0 : row; +assert row.key != null && row.link != 0 && row.expireTime != 0 : row; -row.key.partition(partId); +GridCacheVersion obsoleteVer = null; -if (pendingTree.removex(row)) { if (obsoleteVer == null) obsoleteVer = cctx.cache().nextVersion(); -GridCacheEntryEx e1 = cctx.cache().entryEx(row.key); +GridCacheEntryEx entry = cctx.cache().entryEx(row.key instanceof KeyCacheObjectImpl +? new ExpiredKeyCacheObject((KeyCacheObjectImpl)row.key, row.expireTime, row.link) : row.key); -if (e1 != null) -c.apply(e1, obsoleteVer); +if (entry != null) +c.apply(entry, obsoleteVer); } -cleared++; +cleared += rows.size(); } -while (cur.next()); +while (amount < 0 || cleared < amount); Review Comment: What is this condition for ? `amount < 0` -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21257 Introduce REST API for viewing partition states [ignite-3]
ibessonov merged PR #3614: URL: https://github.com/apache/ignite-3/pull/3614 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-22077 Add spring 6 support to spring session extension [ignite-extensions]
shnus opened a new pull request, #262: URL: https://github.com/apache/ignite-extensions/pull/262 (no 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22062 RO transaction does not close cursor when exception is thrown [ignite-3]
vldpyatkov merged PR #3620: URL: https://github.com/apache/ignite-3/pull/3620 -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-21937 Sql. Cover SQL F041-05 feature by tests [ignite-3]
zstan opened a new pull request, #3643: URL: https://github.com/apache/ignite-3/pull/3643 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- Change summary where - 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
lowka commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574200892 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: Maybe It is better to make it impossible to create invalid statements ? (statements with negative page size, negative timeouts, etc.) -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-18647 Sql. Implement missed Statement and StatementBuilder methods. [ignite-3]
lowka commented on code in PR #3626: URL: https://github.com/apache/ignite-3/pull/3626#discussion_r1574200892 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java: ## @@ -327,11 +322,12 @@ public CompletableFuture> executeAsync( private CompletableFuture> executeAsyncInternal( @Nullable Transaction transaction, -String query, -int pageSize, +Statement statement, @Nullable Object... arguments ) { -assert pageSize > 0 : pageSize; +assert statement.pageSize() >= 0 : statement.pageSize(); Review Comment: Maybe It is better to make it impossible to create invalid statements ? (negative page size, negative timeout etc.) -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-21938 Sql. Cover SQL F041-07 feature by tests [ignite-3]
zstan opened a new pull request, #3642: URL: https://github.com/apache/ignite-3/pull/3642 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- Change summary where - 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org