SQL: Add xDBC and CLI support. QA CSV specs #68966

bpintea · 2021-02-15T07:41:53Z

This adds support for the xDBC and CLI clients.
Based on these, the CsvJdbc-based QA tests are also added.

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

Adds a REST test that generates all possible path combination of a JSON object, all generating values to be retrieved with ARRAY().

…/multivalue-clients

bpintea · 2021-02-15T14:07:31Z

@elasticmachine run elasticsearch-ci/2

bpintea · 2021-02-15T14:42:11Z

@elasticmachine run elasticsearch-ci/2

…/multivalue-clients

Correct timezone formatting pattern.

elasticmachine · 2021-02-15T17:01:20Z

Pinging @elastic/es-ql (Team:QL)

matriv

Looks good!! Had a first pass and left some mostly minor comments.

matriv · 2021-02-15T17:29:41Z

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/stats/VerifierMetricsTests.java

    public void testTwoQueriesExecuted() {
        Metrics metrics = new Metrics();
-        Verifier verifier = new Verifier(metrics);
+        Verifier verifier = new Verifier(metrics, SqlTestUtils.TEST_CFG.version());


static import?

matriv · 2021-02-15T17:30:02Z

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

        EsIndex test = new EsIndex("test", mapping);
        IndexResolution getIndexResult = IndexResolution.valid(test);
-        analyzer = new Analyzer(SqlTestUtils.TEST_CFG, new SqlFunctionRegistry(), getIndexResult, new Verifier(new Metrics()));
+        analyzer = new Analyzer(SqlTestUtils.TEST_CFG, new SqlFunctionRegistry(), getIndexResult,


static import of TEST_CFG?

matriv · 2021-02-15T17:30:26Z

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerRunTests.java

        EsIndex test = new EsIndex("test", mapping);
        getIndexResult = IndexResolution.valid(test);
-        analyzer = new Analyzer(SqlTestUtils.TEST_CFG, functionRegistry, getIndexResult, new Verifier(new Metrics()));
+        analyzer = new Analyzer(SqlTestUtils.TEST_CFG, functionRegistry, getIndexResult,


matriv · 2021-02-15T17:30:39Z

...n/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTablesTests.java

        EsIndex test = new EsIndex("test", mapping);
        Analyzer analyzer = new Analyzer(SqlTestUtils.TEST_CFG, new FunctionRegistry(), IndexResolution.valid(test),
-                                         new Verifier(new Metrics()));
+                                         new Verifier(new Metrics(), SqlTestUtils.TEST_CFG.version()));


matriv · 2021-02-15T17:31:47Z

...in/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java

+            }
+        }
+    }
+


minor: new lines

matriv · 2021-02-15T17:32:57Z

.../src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java


    private String error(IndexResolution getIndexResult, String sql) {
-        Analyzer analyzer = new Analyzer(TEST_CFG, new SqlFunctionRegistry(), getIndexResult, new Verifier(new Metrics()));
+        Analyzer analyzer = new Analyzer(TEST_CFG, new SqlFunctionRegistry(), getIndexResult, new Verifier(new Metrics(),


matriv · 2021-02-15T17:33:01Z

.../src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java


    private LogicalPlan accept(IndexResolution resolution, String sql) {
-        Analyzer analyzer = new Analyzer(TEST_CFG, new SqlFunctionRegistry(), resolution, new Verifier(new Metrics()));
+        Analyzer analyzer = new Analyzer(TEST_CFG, new SqlFunctionRegistry(), resolution, new Verifier(new Metrics(),


Fixed hopefully everywhere (except here, where it was already statically imported and used)

matriv · 2021-02-15T17:41:34Z

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java

        }
    }

+    private static List<String> splitCsvLine(String line) {


Have you checked if org.relique.jdbc.csv (which we use already) has this functionality?

They do have the functionality, but don't export it.

matriv · 2021-02-15T17:43:16Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java

+    }
+
+    static boolean isArray(EsType type) {
+        return type.getName().endsWith("_ARRAY");


Couldn't we have a flag on the EsType for that instead of checking the name?

Right, thanks, fixed to check against Types.ARRAY.

matriv · 2021-02-15T17:44:19Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java

-        throw new SQLFeatureNotSupportedException("Array not supported");
+        EsType type = columnType(columnIndex);
+        if (isArray(type) == false) {
+            throw new SQLException("Cannot get an array for the column [" + columnIndex + "] of type [" + type.getName() + "]");


Maybe Cannot get column ... of type ... as array?

Better, fixed, thx.

matriv · 2021-02-15T19:08:18Z

Can we have some test with prepared statement where an array is set to a query param? (Of course param currently can only be in the SELECT clause).

If it's not in the scope of this PR to have this, then let's have a simple test that tests the Exception thrown.

- static imports - exception message rephrasing - better array type checking.

bpintea · 2021-02-16T07:50:15Z

Can we have some test with prepared statement where an array is set to a query param? (Of course param currently can only be in the SELECT clause).

Array parameters are indeed out of scope atp (just like literals or constructors).

If it's not in the scope of this PR to have this, then let's have a simple test that tests the Exception thrown.

Not sure how to go about this:

passing a JSON array as parameter will trigger a parsing "regular" exception (in the regular request parser); so this is partially covered.
passing a scalar with an _array type (to smth like SELECT ?) will have the type be simply ignored. This is the case btw with intervals too. We can add a filter here, I guess, but wondering why hasn't it already been added, to restrict it to allowed types.

matriv · 2021-02-16T07:56:16Z

passing a scalar with an _array type (to smth like SELECT ?) will have the type be simply ignored. This is the case btw with intervals too. We can add a filter here, I guess, but wondering why hasn't it already been added, to restrict it to allowed types.

I meant to have a test for stmt.setArray() that currently should throw an exception (unsupported type).

astefan

Left a first round of reviews, after going through roughly half of the code changes.

astefan · 2021-02-16T10:05:57Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+        if (index < 0 || index > Integer.MAX_VALUE) {
+            throw new SQLException("Index value [" + index + "] out of range [0, " + Integer.MAX_VALUE + "]");
+        }
+        int index0 = (int) index - 1; // 0-based index


index0 initialization could be moved after the count < 0 check.

astefan · 2021-02-16T10:27:22Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+        if (count < 0) {
+            throw new SQLException("Illegal negative count [" + count + "]");
+        }
+        int available = index0 < values.size() ? values.size() - index0 : 0;


Can you double check the logic here, because I'm not sure it's correct. From what I can see index can be 0. If it is 0 and there are no values in the array values this means the Arrays.copyOfRange will act on an array starting at index -1 until -1 + Math.min(0, 1) and the call to copyOfRange will throw an exception for the negative to index. I think index should be checked at the start of this method with the value 1 (getArray javadoc mentions index the array index of the first element to retrieve;the first element is at index 1).

From what I can see index can be 0.

Right, it could, but should not, as it is 1-based index.

I think index should be checked at the start of this method with the value 1

Correct. The out of range exception you exemplified below was triggered due to the incorrect initial check, index < 0, which is now corrected to < 1 (I also assumed it's 0-based... as an array, so wrote the check, then confronted with the docs, but missed to fix the check :-/).

astefan · 2021-02-16T10:38:03Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java

+            : type;
+    }
+
+    static EsType arrayOf(EsType type) throws SQLException {


Why is this method here, since it's only used in TypeConverterTests? Wouldn't have a better place in that test class? It is because ENUM_NAME_TO_TYPE is local to TypeUtils?

It is because ENUM_NAME_TO_TYPE is local to TypeUtils?

Yes.

(TypeUtils#isString() is btw also only used once, in JdbcPreparedStatement, with no TypeUtils dependencies.)

astefan · 2021-02-16T10:43:20Z

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcArrayTests.java

+    static final List<EsType> ARRAY_TYPES = Arrays.stream(EsType.values()).filter(TypeUtils::isArray).collect(Collectors.toList());
+
+    public void testMetaData() throws Exception {
+        EsType arrayType = randomFrom(ARRAY_TYPES);


Here I would have iterated through all of the array types and assert the checks. I don't think it's worth randomizing this.

Exhaustive-ised.

astefan · 2021-02-16T10:46:28Z

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcArrayTests.java

+        Object[] contained = (Object[]) array.getArray(4, 3);
+        assertEquals(asList(3, 4, 5), asList(contained));
+
+        Object[] overlapping = (Object[]) array.getArray(9, 3);


In this test class my assumption about getArray with a 0 as index parameter is confirmed if I do Object[] bla = (Object[]) array.getArray(0, 1);. The call fails with ArrayIndexOutOfBoundsException.

Nice, thanks, fixed.

astefan · 2021-02-16T11:04:12Z

x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java


+    static <T> Set<T> randomSet(int minSetSize, int maxSetSize, Supplier<T> valueConstructor) {
+        final int size = randomIntBetween(minSetSize, maxSetSize);
+        Set<T> set = new HashSet<>();


Why not putting a size to the HashSet constructor, since it's already known. To help with this a bit, we have in CollectionUtils the method mapSize.

CollectionUtils is in ql, which isn't a dependency. Not sure if optimising a random-size-driven test would be worth, but still, I could add the method ("formula") here if you think it'd be better that way.

Not worth adding ql as a dependency for this.

- fix index bounds check and add tests for it; - imporove testing; - minor refactorings.

bpintea · 2021-02-16T12:30:03Z

I meant to have a test for stmt.setArray() that currently should throw an exception (unsupported type).

I see. Makes sense, I've added it.

…/multivalue-clients

astefan · 2021-02-16T12:43:55Z

.../plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java

+            List<Integer> expected = asList(ints);
+            expected.sort(Integer::compare);
+            List<?> actual = asList((Object[]) intArray.getArray());
+            actual.sort(Comparator.comparing(x -> ((Integer) x)));


Suggested change

actual.sort(Comparator.comparing(x -> ((Integer) x)));

actual.sort(Comparator.comparingInt(x -> (Integer) x));

Applied, thanks.

astefan · 2021-02-16T12:44:40Z

.../plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java

+
+            strings.sort(String::compareTo);
+            actual = asList((Object[]) strArray.getArray());
+            actual.sort(Comparator.comparing(x -> ((String) x)));


Suggested change

actual.sort(Comparator.comparing(x -> ((String) x)));

actual.sort(Comparator.comparing(x -> (String) x));

astefan · 2021-02-16T12:55:01Z

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java

+            builder.append("}").append("\n");
+        });
+
+        request = new Request("POST", "/" + index + "/_bulk?refresh=wait_for");


I'm confused about refresh=wait_for and the parameter below request.addParameter("refresh", "true"). Aren't these overriding one another?

My understanding of the two is that refresh=true initiates a refresh right away, while refresh=wait_for will cause the response to only be given once the refresh is finished. I.e. refresh=true speeds up the test.

@astefan thanks for looking further into it. I've replaced ?refresh=wait_for with ?refresh=true (the last parameter being the one that applies) throughout the class.

astefan · 2021-02-16T13:00:08Z

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java

+                }
+            }
+            if (actualObject instanceof Point) {
+                // geo points are loaded form doc values where they are stored as long-encoded values leading


Actually, this statement is not true anymore. We don't use docvalues extraction. We used to, not anymore.

I've removed the comment (but kept the "with-delta" comparison, I guess the fields also returns them from docvalues?)

At the moment, no. Extraction from other sources other than _source is something for the future.

astefan · 2021-02-16T13:03:34Z

.../plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

+    @SuppressWarnings("unchecked")
+    public void testMultiValueDifferentPathsWithArraysAndNulls() throws IOException {
+        ExhaustiveMultiPathMapper<Long> mapper = new ExhaustiveMultiPathMapper<>(randomIntBetween(3, 10));
+        // TODO AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/68979")


astefan · 2021-02-16T13:22:57Z

.../plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

+        }
+
+        Map<String, Object> map() {
+            return map == null ? null : new HashMap<>(map);


Why new HashMap(map) and not returning the map itself?

To allow the caller to modify what's returned and still fetch the original if needed. (Flexibility not really used now, tho.)

If it's not used, I'd say let's keep it simple for the moment and when we'll need it, we'll add the flexibility. My 2c

I've updated ~~the returns to the respective unmodifiable collections~~.

astefan · 2021-02-16T13:28:59Z

.../plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

+                if (val instanceof Map) {
+                    @SuppressWarnings("unchecked")
+                    Map<String, Object> innerMap = (Map<String, Object>) val;
+                    if (randomNonNegativeByte() < 60) {


I think this class needs some comments here and there to help understanding what is it actually doing. For example, what's with if (randomNonNegativeByte() < 60)? Is 60 just a random number you picked up or does it have a deeper meaning?

I've added some comments here and there.
In this case // assuming a linear distribution of the random non-negative byte values (0-255), [access this branch] roughly 1/4 of the time

There are some helper methods already existent that fit into the same ballpark idea. Maybe reuse those, if you think they fit the original intention in this test? rarely(), usually(), frequently().

Thanks. I've switched to usually(), which boost the probability to over 50% from ~25%, but should not make a difference testing-wise.

astefan · 2021-02-16T13:37:20Z

.../plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

+        private List<T> values;
+
+        // the depth excludes the leaf
+        ExhaustiveMultiPathMapper(int depth) {


Why not passing the supplier to the constructor as well and initialize all the internal variables - paths, map, values - in the constructor and from outside you only call the geter methods. The way it is now, the caller of this class needs to call the constructor and, then, call the generate method to populate all the internal variables. And I don't see why it should leave room for trial&error from the user of this class. Restricting everything to the constructor makes the user of the class just call the constructor (to have everything ready) and then get the internal variables values by calling geters.

Makes sense, I've refactored.
(The initial idea was to allow recalling one mapper instance with different value suppliers, but this is superfluous after all.)

- more comments - refactor testing class - remove unnecessary brackets.

…/multivalue-clients

matriv · 2021-02-17T09:31:21Z

...ugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcPreparedStatementTests.java

        assertEquals("Conversion from type [byte[]] to [INTERVAL_DAY_TO_MINUTE] not supported", sqle.getMessage());
    }

+    public void testThrownExceptionWhenSettingArrayValue() throws SQLException {


- refactor the way results are returned in mapper; - reuse existing probability functions. - remove redundant `refresh=wait_for`.

…/multivalue-clients

- undo returning unmodifiable collection in mapper.

…/multivalue-clients

astefan

LGTM

matriv

LGTM, Nice work!

…/multivalue-clients

- adjust test after upstream ticket has been addressed.

Now that 7.13.0 is available, update the release target to that version.

costin

Let a few comments. Great work!

costin · 2021-02-19T09:49:16Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+
+import static org.elasticsearch.xpack.sql.jdbc.TypeUtils.baseType;
+
+public class JdbcArray implements Array {


Should have package visibility to avoid instantiation outside the package.

Right, thanks, fixed.

costin · 2021-02-19T10:42:06Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java

    GEO_SHAPE(                ExtraTypes.GEOMETRY),
-    SHAPE(                    ExtraTypes.GEOMETRY);
+    SHAPE(                    ExtraTypes.GEOMETRY),
+    BOOLEAN_ARRAY(            Types.ARRAY),


Discussed with @bpintea to refactor this to use only a generic ARRAY type.

costin · 2021-02-19T10:45:09Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+
+    @Override
+    public Object getArray(long index, int count, Map<String, Class<?>> map) throws SQLException {
+        if (map == null || map.isEmpty()) {


Small preference, it's better to do the validation and error checking upfront as oppose to last.
That is check if the map is non null and non-empty and throw the exception vs what the way it is right now.

Updated it.
However, I believe we might need to do further updates to stay consistent.

costin · 2021-02-19T10:46:42Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+    }
+
+    @Override
+    public ResultSet getResultSet() throws SQLException {


A follow-up should implement this method - typically by returning a ResultSet that contains two columns - index and value.
A good deal of clients rely on this method since they're used to parsing ResultSet vs getArray().
See JdbcDatabaseMetaData#emptySet for example of creating a dummy ResultSet.

ResultSet methods added in #69512.

costin · 2021-02-19T10:48:55Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+    }
+
+    @Override
+    public void free() throws SQLException {


Once this method is called, all other methods should return an exception which is not the case.
There are no side-effects however this is the mandatory behavior described by the spec:

* After <code>free</code> has been called, any attempt to invoke a * method other than <code>free</code> will result in a <code>SQLException</code> * being thrown. If <code>free</code> is called multiple times, the subsequent * calls to <code>free</code> are treated as a no-op.

Right, on another look, that phrasing is mandatory. Check added.

costin · 2021-02-19T10:52:11Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java

                }
            case IP:
                return v.toString();
+            case BOOLEAN_ARRAY:


Needs to be improved

costin · 2021-02-19T10:52:34Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcArray.java

+    private final EsType type;
+    private final List<?> values;
+
+    public JdbcArray(EsType type, List<?> values) {


Pass in the base type separately instead of determining it from the EsType

Changed to pass the base type.

costin · 2021-02-19T10:54:07Z

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java

        }
    }

+    private static Object extractObject(ResultSet resultSet, int column, boolean fromEs, Class<?> expectedColumnClass) throws SQLException {


A lot of work went in here.

costin · 2021-02-19T10:54:51Z

.../plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

+     * Some of the paths are randomly multiplied:
+     *   {"a": [{"b": {"c": 1}}}, {"b": {"c": 1}}}], ...}
+     */
+    private static class ExhaustiveMultiPathMapper<T> {


And here...

costin · 2021-02-19T10:56:28Z

...plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/VersionCompatibilityChecks.java

+
+public final class VersionCompatibilityChecks {
+
+    public static final SqlVersion INTRODUCING_ARRAY_TYPES = SqlVersion.fromId(V_7_13_0.id);


I wonder if we need to remove INTRODUCING_ since it will be the same for all new features. Then again, we need to differentiate from those that are being removed.

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

Add xDBC and CLI support. QA CSV specs

f6ebe6c

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

bpintea mentioned this pull request Feb 15, 2021

SQL: Add xDBC and CLI support. QA CSV specs #68607

Closed

bpintea added 2 commits February 15, 2021 14:10

Improve array testing

190860e

Adds a REST test that generates all possible path combination of a JSON object, all generating values to be retrieved with ARRAY().

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

663382a

…/multivalue-clients

bpintea added 2 commits February 15, 2021 16:22

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

e518fa2

…/multivalue-clients

Test fix

73eacba

Correct timezone formatting pattern.

bpintea added :Analytics/SQL SQL querying v8.0.0 labels Feb 15, 2021

bpintea marked this pull request as ready for review February 15, 2021 17:01

elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Feb 15, 2021

bpintea requested review from astefan, costin, matriv and palesz February 15, 2021 17:01

matriv reviewed Feb 15, 2021

View reviewed changes

Address review comments

06f4e5e

- static imports - exception message rephrasing - better array type checking.

astefan requested changes Feb 16, 2021

View reviewed changes

Address review comments

ea020b0

- fix index bounds check and add tests for it; - imporove testing; - minor refactorings.

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

b7055a2

…/multivalue-clients

astefan reviewed Feb 16, 2021

View reviewed changes

bpintea added 2 commits February 16, 2021 20:03

Adress review comments

623d579

- more comments - refactor testing class - remove unnecessary brackets.

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

78076c9

…/multivalue-clients

matriv reviewed Feb 17, 2021

View reviewed changes

bpintea added 4 commits February 17, 2021 15:15

Address review feedback

93cf78b

- refactor the way results are returned in mapper; - reuse existing probability functions. - remove redundant `refresh=wait_for`.

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

1e33046

…/multivalue-clients

Test fix

323af93

- undo returning unmodifiable collection in mapper.

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

c4d0e01

…/multivalue-clients

bpintea requested a review from astefan February 17, 2021 17:51

astefan approved these changes Feb 18, 2021

View reviewed changes

matriv approved these changes Feb 18, 2021

View reviewed changes

bpintea added 3 commits February 19, 2021 08:34

Merge remote-tracking branch 'upstream/feat/sql-multivalue' into feat…

c9cdcd3

…/multivalue-clients

Adjust test

abe2bc2

- adjust test after upstream ticket has been addressed.

Update feature-introducing version

9dae2dc

Now that 7.13.0 is available, update the release target to that version.

bpintea merged commit 496a842 into elastic:feat/sql-multivalue Feb 19, 2021

bpintea deleted the feat/multivalue-clients branch February 19, 2021 09:51

costin reviewed Feb 19, 2021

View reviewed changes

bpintea mentioned this pull request Feb 24, 2021

SQL: Export array values through result sets #69512

Merged

bpintea mentioned this pull request Mar 11, 2021

SQL: add multivalue field support (arrays) #68066

Closed

6 tasks

bpintea added a commit that referenced this pull request Mar 16, 2021

SQL: Add xDBC and CLI support. QA CSV specs (#68966)

7ee3c0d

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

bpintea mentioned this pull request Mar 17, 2021

SQL: Replace JDBC qualified array types with a generic one #70511

Merged

bpintea added a commit that referenced this pull request Mar 25, 2021

SQL: Add xDBC and CLI support. QA CSV specs (#68966)

6f85936

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

pugnascotia added the >enhancement label Mar 31, 2021

bpintea added a commit that referenced this pull request May 20, 2021

SQL: Add xDBC and CLI support. QA CSV specs (#68966)

fca432e

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

bpintea added a commit to bpintea/elasticsearch that referenced this pull request May 20, 2021

SQL: Add xDBC and CLI support. QA CSV specs (elastic#68966)

f1128a2

This adds support for the xDBC and CLI clients. Based on those, the CsvJdbc-based QA tests are also integrated.

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

	actual.sort(Comparator.comparing(x -> ((Integer) x)));
	actual.sort(Comparator.comparingInt(x -> (Integer) x));

	actual.sort(Comparator.comparing(x -> ((String) x)));
	actual.sort(Comparator.comparing(x -> (String) x));


		import static org.elasticsearch.xpack.sql.jdbc.TypeUtils.baseType;

		public class JdbcArray implements Array {


		public final class VersionCompatibilityChecks {

		public static final SqlVersion INTRODUCING_ARRAY_TYPES = SqlVersion.fromId(V_7_13_0.id);

SQL: Add xDBC and CLI support. QA CSV specs #68966

SQL: Add xDBC and CLI support. QA CSV specs #68966

Uh oh!

Conversation

bpintea commented Feb 15, 2021

Uh oh!

bpintea commented Feb 15, 2021

Uh oh!

bpintea commented Feb 15, 2021

Uh oh!

elasticmachine commented Feb 15, 2021

Uh oh!

matriv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matriv commented Feb 15, 2021

Uh oh!

bpintea commented Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matriv commented Feb 16, 2021

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpintea Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

bpintea commented Feb 16, 2021 •

edited

Loading

bpintea Feb 16, 2021 •

edited

Loading