You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/12/15 10:42:34 UTC
[GitHub] [ignite-3] AMashenkov opened a new pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
AMashenkov opened a new pull request #510:
URL: https://github.com/apache/ignite-3/pull/510
https://issues.apache.org/jira/browse/IGNITE-16115
--
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
[GitHub] [ignite-3] korlov42 commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r775784892
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -66,62 +66,52 @@
/**
* Gets a nullable value associated with the given key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
* @return Wrapped nullable value or {@code null}, if it does not exist.
*/
- default NullableValue<V> getNullable(@Nullable Transaction tx, K key) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ NullableValue<V> getNullable(@Nullable Transaction tx, @NotNull K key);
/**
* Gets a nullable value associated with the given key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
* @return Future representing pending completion of the operation.
* @see #getNullable(Transaction, Object)
*/
- default @NotNull CompletableFuture<NullableValue<V>> getNullableAsync(@Nullable Transaction tx, K key) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ @NotNull CompletableFuture<NullableValue<V>> getNullableAsync(@Nullable Transaction tx, @NotNull K key);
/**
* Gets a value associated with the given key if exists, returns {@code defaultValue} otherwise.
*
- * <p>Note: method has same semantic as {@link #get(Transaction, Object)} with regard to {@code null} values.
+ * <p>Note: result may be {@code null} if null-values are allowed, and the {@code null} is associated with the key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
+ * @param defaultValue Default value that. The value cannot be {@code null}.
* @return Value or {@code defaultValue}, if does not exist.
* @throws IllegalStateException If value for the key exists, and it is {@code null}.
*/
- default V getOrDefault(@Nullable Transaction tx, K key, V defaultValue) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ V getOrDefault(@Nullable Transaction tx, @NotNull K key, @NotNull V defaultValue);
/**
- * Gets a nullable value associated with the given key.
+ * Gets a value associated with the given key if exists, returns {@code defaultValue} otherwise.
Review comment:
```suggestion
* Returns a value associated with the given key if exists, returns {@code defaultValue} otherwise.
```
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -223,7 +213,7 @@ default V getOrDefault(@Nullable Transaction tx, K key, V defaultValue) {
/**
* Asynchronously puts value associated with given key into the table if not exists.
*
- * @param tx The transaction or {@code null} to auto commit.
Review comment:
AFAIK all descriptions should be aligned within a group (all `param`s descriptions, all `throw`s descriptions). Could you please fix this everywhere in this PR?
##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/SchemaRegistryImpl.java
##########
@@ -145,40 +146,48 @@ public Row resolve(BinaryRow row) {
/** {@inheritDoc} */
@Override
- public Collection<Row> resolve(Collection<BinaryRow> rows) {
+ public Collection<Row> resolve(Collection<BinaryRow> binaryRows) {
final SchemaDescriptor curSchema = waitLatestSchema();
- return rows.stream().map(row -> resolveInternal(row, curSchema)).collect(toList());
+ List<Row> rows = new ArrayList<>(binaryRows.size());
+
+ for (BinaryRow r : binaryRows) {
+ if (r != null) {
+ rows.add(resolveInternal(r, curSchema));
+ }
+ }
+
+ return rows;
}
/**
- * Resolves a schema for row.
- * The method is optimal when the latest schema is already gotten.
+ * Resolves a schema for row. The method is optimal when the latest schema is already got.
*
- * @param row Binary row.
+ * @param row Binary row.
* @param curSchema The latest available local schema.
- * @return Schema-aware rows.
+ * @return Schema-aware row or {@code null} of the given row is null.
Review comment:
```suggestion
* @return Schema-aware row or {@code null} if the given row is null.
```
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -66,62 +66,52 @@
/**
* Gets a nullable value associated with the given key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
* @return Wrapped nullable value or {@code null}, if it does not exist.
*/
- default NullableValue<V> getNullable(@Nullable Transaction tx, K key) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ NullableValue<V> getNullable(@Nullable Transaction tx, @NotNull K key);
/**
* Gets a nullable value associated with the given key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
* @return Future representing pending completion of the operation.
* @see #getNullable(Transaction, Object)
*/
- default @NotNull CompletableFuture<NullableValue<V>> getNullableAsync(@Nullable Transaction tx, K key) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ @NotNull CompletableFuture<NullableValue<V>> getNullableAsync(@Nullable Transaction tx, @NotNull K key);
/**
* Gets a value associated with the given key if exists, returns {@code defaultValue} otherwise.
*
- * <p>Note: method has same semantic as {@link #get(Transaction, Object)} with regard to {@code null} values.
+ * <p>Note: result may be {@code null} if null-values are allowed, and the {@code null} is associated with the key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
+ * @param defaultValue Default value that. The value cannot be {@code null}.
* @return Value or {@code defaultValue}, if does not exist.
* @throws IllegalStateException If value for the key exists, and it is {@code null}.
*/
- default V getOrDefault(@Nullable Transaction tx, K key, V defaultValue) {
- //TODO: to be implemented https://issues.apache.org/jira/browse/IGNITE-16115
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ V getOrDefault(@Nullable Transaction tx, @NotNull K key, @NotNull V defaultValue);
/**
- * Gets a nullable value associated with the given key.
+ * Gets a value associated with the given key if exists, returns {@code defaultValue} otherwise.
*
- * <p>Note: method has same semantic as {@link #get(Transaction, Object)} with regard to {@code null} values.
+ * <p>Note: result may be {@code null} if null-values are allowed, and the {@code null} is associated with the key.
*
- * @param tx The transaction or {@code null} to auto commit.
+ * @param tx The transaction or {@code null} to auto commit.
* @param key A key which associated the value is to be returned. The key cannot be {@code null}.
+ * @param defaultValue Default value that. The value cannot be {@code null}.
Review comment:
looks like something missing in this 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
[GitHub] [ignite-3] agura commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r786227662
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/KeyValueViewOperationsSimpleSchemaTest.java
##########
@@ -84,7 +86,7 @@ public void put() {
assertEquals(22L, tbl.get(null, 1L));
// Remove KV pair.
- tbl.put(null, 1L, null);
+ tbl.remove(null, 1L);
Review comment:
Nevertheless, what is behavior of `put(tx, key, null)` invocation? There are at least two cases: value is a nullable column and values is a not null column. Method's contract doesn't throw any exception and doesn't document how it behaves.
--
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
[GitHub] [ignite-3] agura commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r772856756
##########
File path: modules/api/src/main/java/org/apache/ignite/lang/NullableValue.java
##########
@@ -26,11 +27,50 @@
* @param <T> Value type.
* @see org.apache.ignite.table.KeyValueView#getNullable(Transaction, Object)
Review comment:
I don't think that it is a good idea to refer to a `tx` package from `lang` package. It is the calls of general purpose and it could be used al alternative to `Optional` with relaxed requirement for return value. Better documentation should note it.
##########
File path: modules/api/src/main/java/org/apache/ignite/lang/NullableValue.java
##########
@@ -26,11 +27,50 @@
* @param <T> Value type.
* @see org.apache.ignite.table.KeyValueView#getNullable(Transaction, Object)
*/
-public interface NullableValue<T> {
+public final class NullableValue<T> {
+ /** Wrapped value. */
+ private T value;
+
+ /**
+ * Creates a wrapper for nullable value.
+ *
+ * @param value Value.
+ */
+ public NullableValue(@Nullable T value) {
+ this.value = value;
+ }
+
/**
* Returns wrapped value.
*
* @return Value.
*/
- @Nullable T get();
+ public @Nullable T value() {
Review comment:
Why not just `get`?
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/KeyValueViewOperationsSimpleSchemaTest.java
##########
@@ -84,7 +86,7 @@ public void put() {
assertEquals(22L, tbl.get(null, 1L));
// Remove KV pair.
- tbl.put(null, 1L, null);
+ tbl.remove(null, 1L);
Review comment:
By the way, what is the behavior of the `put` method after this commit in the case of the `null` value?
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -41,8 +41,8 @@
* <p>Note: If the value mapper implies a value can be {@code null}, then a suitable method
* {@link #getNullable(Transaction, Object)} must be used instead.
*
- * @param tx The transaction or {@code null} to auto commit.
- * @param key A key which associated the value is to be returned. The key cannot be {@code null}.
+ * @param tx The transaction or {@code null} to auto commit.
+ * @param key A key which associated the value is to be returned. The key cannot be {@code null}.
* @return Value or {@code null}, if it does not exist.
* @throws IllegalStateException If value for the key exists, and it is {@code null}.
Review comment:
IMHO, it should be a special exception instead of a just `IllegalStateException`. it is true for all methods which assume that value can't be `null`.
##########
File path: modules/api/src/main/java/org/apache/ignite/lang/NullableValue.java
##########
@@ -26,11 +27,50 @@
* @param <T> Value type.
* @see org.apache.ignite.table.KeyValueView#getNullable(Transaction, Object)
*/
-public interface NullableValue<T> {
+public final class NullableValue<T> {
+ /** Wrapped value. */
+ private T value;
+
+ /**
+ * Creates a wrapper for nullable value.
+ *
+ * @param value Value.
+ */
+ public NullableValue(@Nullable T value) {
+ this.value = value;
+ }
+
/**
* Returns wrapped value.
*
* @return Value.
*/
- @Nullable T get();
+ public @Nullable T value() {
+ return value;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ NullableValue<?> that = (NullableValue<?>) o;
+ return Objects.equals(value, that.value);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ return Objects.hash(value);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ return "NullableValue{value=" + value + '}';
Review comment:
Does it conform to Apache Ignite log formats? It seems that no.
##########
File path: modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientKeyValueBinaryView.java
##########
@@ -85,6 +86,46 @@ public Tuple get(@Nullable Transaction tx, @NotNull Tuple key) {
Collections.emptyMap());
}
+ /**
+ * Method throws UnsupportedOperationException, unconditionally.
+ *
+ * <p>Binary view doesn't support the operation because there is no ambiguity, {@code null} value means no rows found in the table for
+ * the given key.
+ */
+ @Override
+ public NullableValue<Tuple> getNullable(@Nullable Transaction tx, @NotNull Tuple key) {
+ throw new UnsupportedOperationException("Binary view doesn't allow null values. Please, use get(key) method instead.");
Review comment:
Unmaintainable message in exception. The part about using `get(key)` could easily get not actual. I think this part should be part of JavaDoc, not the exception message.
The same is true for other methods with such a message.
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/KeyValueViewImpl.java
##########
@@ -80,11 +81,52 @@ public V get(@Nullable Transaction tx, @NotNull K key) {
/** {@inheritDoc} */
@Override
- public @NotNull
- CompletableFuture<V> getAsync(@Nullable Transaction tx, @NotNull K key) {
+ public @NotNull CompletableFuture<V> getAsync(@Nullable Transaction tx, @NotNull K key) {
+ BinaryRow keyRow = marshal(Objects.requireNonNull(key));
+
+ return tbl.get(keyRow, (InternalTransaction) tx).thenApply(row -> {
+ if (row == null) {
+ return null;
+ }
+
+ V v = unmarshalValue(row);
+
+ if (v == null) {
+ throw new IllegalStateException("Got unexpected 'null' value. Please, use 'getNullable' method instead.");
Review comment:
Unmaintainable message. Just properly document it in javadoc.
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/KeyValueBinaryViewImpl.java
##########
@@ -70,11 +71,47 @@ public Tuple get(@Nullable Transaction tx, @NotNull Tuple key) {
public @NotNull CompletableFuture<Tuple> getAsync(@Nullable Transaction tx, @NotNull Tuple key) {
Objects.requireNonNull(key);
- Row keyRow = marshal(key, null); // Convert to portable format to pass TX/storage layer.
+ Row keyRow = marshal(key, null);
- return tbl.get(keyRow, (InternalTransaction) tx) // Load async.
- .thenApply(this::wrap) // Binary -> schema-aware row
- .thenApply(TableRow::valueTuple); // Narrow to value.
+ return tbl.get(keyRow, (InternalTransaction) tx).thenApply(this::unmarshal);
+ }
+
+ /**
+ * Method throws UnsupportedOperationException, unconditionally.
+ *
+ * <p>Binary view doesn't support the operation because there is no ambiguity, {@code null} value means no rows found in the table for
+ * the given key.
+ */
+ @Override
+ public NullableValue<Tuple> getNullable(@Nullable Transaction tx, @NotNull Tuple key) {
+ throw new UnsupportedOperationException("Binary view doesn't allow null values. Please, use get(key) method instead.");
Review comment:
Unmaintainable message in exception. See `ClientKeyValueBinaryView` comments.
##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/SchemaRegistryImpl.java
##########
@@ -148,12 +149,11 @@ public Row resolve(BinaryRow row) {
public Collection<Row> resolve(Collection<BinaryRow> rows) {
final SchemaDescriptor curSchema = waitLatestSchema();
- return rows.stream().map(row -> resolveInternal(row, curSchema)).collect(toList());
+ return rows.stream().filter(Objects::nonNull).map(row -> resolveInternal(row, curSchema)).collect(toList());
Review comment:
Please, rewrite this line with code that does not use Stream API. This code potentially is on a hot path.
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/KeyValueViewImpl.java
##########
@@ -80,11 +81,52 @@ public V get(@Nullable Transaction tx, @NotNull K key) {
/** {@inheritDoc} */
@Override
- public @NotNull
- CompletableFuture<V> getAsync(@Nullable Transaction tx, @NotNull K key) {
+ public @NotNull CompletableFuture<V> getAsync(@Nullable Transaction tx, @NotNull K key) {
+ BinaryRow keyRow = marshal(Objects.requireNonNull(key));
+
+ return tbl.get(keyRow, (InternalTransaction) tx).thenApply(row -> {
+ if (row == null) {
+ return null;
+ }
+
+ V v = unmarshalValue(row);
+
+ if (v == null) {
+ throw new IllegalStateException("Got unexpected 'null' value. Please, use 'getNullable' method instead.");
Review comment:
Also, I believe, some special exception should be here.
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/KeyValueBinaryViewImpl.java
##########
@@ -240,7 +273,7 @@ public boolean remove(@Nullable Transaction tx, @NotNull Tuple key, @NotNull Tup
}
return tbl.deleteAll(keyRows, (InternalTransaction) tx)
- .thenApply(t -> t.stream().filter(Objects::nonNull).map(this::wrap).map(TableRow::valueTuple).collect(Collectors.toList()));
+ .thenApply(t -> t.stream().filter(Objects::nonNull).map(this::unmarshal).collect(Collectors.toList()));
Review comment:
Please, rewrite it with code that does not use Stream API. This code potentially is on a hot path.
--
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
[GitHub] [ignite-3] tledkov-gridgain commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r800481830
##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/SchemaRegistryImpl.java
##########
@@ -151,40 +152,48 @@ public Row resolve(BinaryRow row, SchemaDescriptor schemaDescriptor) {
/** {@inheritDoc} */
@Override
- public Collection<Row> resolve(Collection<BinaryRow> rows) {
+ public Collection<Row> resolve(Collection<BinaryRow> binaryRows) {
final SchemaDescriptor curSchema = waitLatestSchema();
- return rows.stream().map(row -> resolveInternal(row, curSchema)).collect(toList());
+ List<Row> rows = new ArrayList<>(binaryRows.size());
+
+ for (BinaryRow r : binaryRows) {
+ if (r != null) {
Review comment:
Which case produces `r == null`?
--
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
[GitHub] [ignite-3] agura commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r786227662
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/KeyValueViewOperationsSimpleSchemaTest.java
##########
@@ -84,7 +86,7 @@ public void put() {
assertEquals(22L, tbl.get(null, 1L));
// Remove KV pair.
- tbl.put(null, 1L, null);
+ tbl.remove(null, 1L);
Review comment:
Hmm... What is a semantic of `put(tx, key, null)` invocation in the case of `KeyValueView` or `RecordView`?
--
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
[GitHub] [ignite-3] tledkov-gridgain commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r800481830
##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/SchemaRegistryImpl.java
##########
@@ -151,40 +152,48 @@ public Row resolve(BinaryRow row, SchemaDescriptor schemaDescriptor) {
/** {@inheritDoc} */
@Override
- public Collection<Row> resolve(Collection<BinaryRow> rows) {
+ public Collection<Row> resolve(Collection<BinaryRow> binaryRows) {
final SchemaDescriptor curSchema = waitLatestSchema();
- return rows.stream().map(row -> resolveInternal(row, curSchema)).collect(toList());
+ List<Row> rows = new ArrayList<>(binaryRows.size());
+
+ for (BinaryRow r : binaryRows) {
+ if (r != null) {
Review comment:
Which case produces `r == null`
--
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
[GitHub] [ignite-3] AMashenkov commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r773053894
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/KeyValueViewOperationsSimpleSchemaTest.java
##########
@@ -84,7 +86,7 @@ public void put() {
assertEquals(22L, tbl.get(null, 1L));
// Remove KV pair.
- tbl.put(null, 1L, null);
+ tbl.remove(null, 1L);
Review comment:
`put` accepts `null` values.
this case covered in getOrDefault() and getNullable() 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
[GitHub] [ignite-3] AMashenkov commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r781193459
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -223,7 +213,7 @@ default V getOrDefault(@Nullable Transaction tx, K key, V defaultValue) {
/**
* Asynchronously puts value associated with given key into the table if not exists.
*
- * @param tx The transaction or {@code null} to auto commit.
Review comment:
Is there any proof?
I've just got opposite opinion here [1] from @agura
> Please, prefer not aligned comments relatively parameters name. It is hard to support.
[1] https://github.com/apache/ignite-3/pull/326#discussion_r772402632
--
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
[GitHub] [ignite-3] AMashenkov commented on a change in pull request #510: IGNITE-16115: Implement getOrDefault and getNullable key-value operations.
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #510:
URL: https://github.com/apache/ignite-3/pull/510#discussion_r781193459
##########
File path: modules/api/src/main/java/org/apache/ignite/table/KeyValueView.java
##########
@@ -223,7 +213,7 @@ default V getOrDefault(@Nullable Transaction tx, K key, V defaultValue) {
/**
* Asynchronously puts value associated with given key into the table if not exists.
*
- * @param tx The transaction or {@code null} to auto commit.
Review comment:
Is there any proof?
I've just got opposite opinion here [1]
[1] https://github.com/apache/ignite-3/pull/326#discussion_r772402632
--
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