korlov42 commented on a change in pull request #484:
URL: https://github.com/apache/ignite-3/pull/484#discussion_r762936817
##########
File path:
modules/runner/src/integrationTest/java/org/apache/ignite/internal/calcite/ItMixedQueriesTest.java
##########
@@ -38,7 +38,6 @@
/**
* Group of tests that still has not been sorted out. It’s better to avoid
extending this class with new tests.
*/
-@Disabled("https://issues.apache.org/jira/browse/IGNITE-15655";)
Review comment:
this ticket still `IN PROGRESS`
##########
File path:
modules/runner/src/integrationTest/java/org/apache/ignite/internal/calcite/ItDataTypesTest.java
##########
@@ -22,20 +22,18 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
* Test SQL data types.
*/
-@Disabled("https://issues.apache.org/jira/browse/IGNITE-15107";)
public class ItDataTypesTest extends AbstractBasicIntegrationTest {
/**
* Before all.
*/
@Test
public void testUnicodeStrings() {
- sql("CREATE TABLE string_table(key int primary key, val varchar)");
+ sql("CREATE TABLE string_table(key int primary key, val varchar) with
partitions=10,replicas=2");
Review comment:
perhaps, it would be better to change default in
`TableConfigurationSchema` and file a ticket to revert
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
##########
@@ -598,4 +598,14 @@ public static Path atomicMoveFile(Path sourcePath, Path
targetPath, @Nullable Ig
return success;
}
+
+ /**
+ * Tests if given string is {@code null} or empty.
+ *
+ * @param s String to test.
+ * @return Whether or not the given string is {@code null} or empty.
+ */
+ public static boolean isNullOrEmpty(@Nullable String s) {
Review comment:
```suggestion
public static boolean nullOrEmpty(@Nullable String s) {
```
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/util/ArrayUtils.java
##########
@@ -296,6 +299,21 @@ public static boolean nullOrEmpty(boolean[] arr) {
return nullOrEmpty(vals) ? Collections.emptyList() :
Arrays.asList(vals);
}
+ /**
+ * Converts array to {@link Set}.
+ *
+ * <p>Note that unlike {@link Arrays#asList(Object[])}, this method is
{@code null}-safe. If {@code null} is passed in, then empty set
+ * will be returned.
+ *
+ * @param vals Array of values
+ * @param <T> Array type.
+ * @return {@link Set} instance for input array.
+ */
+ @SafeVarargs
+ public static <T> Set<T> asSet(@Nullable T... vals) {
+ return nullOrEmpty(vals) ? Collections.emptySet() :
Stream.of(vals).collect(Collectors.toSet());
Review comment:
`Collections.emptySet()` return an immutable set, whereas
`Stream.of(vals).collect(Collectors.toSet())` returns mutable. To me it would
be better to keep semantic consistent across the whole range of arguments. Thus
I propose to use `Set.of(...)` in case of non empty array.
BTW the very same issue exists for `asList` method. Could you please fix
this as well?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/util/ArrayUtils.java
##########
@@ -296,6 +299,21 @@ public static boolean nullOrEmpty(boolean[] arr) {
return nullOrEmpty(vals) ? Collections.emptyList() :
Arrays.asList(vals);
}
+ /**
+ * Converts array to {@link Set}.
+ *
+ * <p>Note that unlike {@link Arrays#asList(Object[])}, this method is
{@code null}-safe. If {@code null} is passed in, then empty set
+ * will be returned.
+ *
+ * @param vals Array of values
Review comment:
missed dot
##########
File path:
modules/api/src/main/java/org/apache/ignite/schema/definition/builder/ColumnDefinitionBuilder.java
##########
@@ -25,18 +25,13 @@
*/
public interface ColumnDefinitionBuilder extends SchemaObjectBuilder {
/**
- * Mark column as nullable.
+ * Set nullable attribute.
Review comment:
```suggestion
* Set nullability attribute.
```
##########
File path:
modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaConfigurationTest.java
##########
@@ -40,12 +41,14 @@ public void testInitialSchema() {
builder
.columns(
- // Declaring columns in user order.
+ // Declaring columns in user order.
+ Arrays.asList(
Review comment:
looks like `Arrays.asList(` could be omitted
##########
File path:
modules/runner/src/integrationTest/java/org/apache/ignite/internal/calcite/ItDataTypesTest.java
##########
@@ -22,20 +22,18 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/**
* Test SQL data types.
*/
-@Disabled("https://issues.apache.org/jira/browse/IGNITE-15107";)
Review comment:
This test should be disabled with
https://issues.apache.org/jira/browse/IGNITE-15655
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
