This is done by Frame for JSON and ColumnValue for Protobufs. For JSON, each value in the row attribute of a frame might be an array instead of a scalar value.

I have noticed in CALCITE-1040, however, that the parsing of scalar or array values is busted for arrays of length 1 with protobuf. Apparently I never wrote up a test for this. I'm currently working on adding an array_value and scalar_value to ColumnValue for protobuf to handle this conversion correctly.

Does this make sense to you?

Unfortunately, it doesn't. In order to submit ExecuteRequest, I need to get a list of TypedValue and I don't see anything in the code that would be parsing or serializing arrays. Do you have an example how to send an array to TypedValue in JSON?

I can't really use the protobuf interface, because it's using the beta version of proto3, which would make my code very hard to install.

In order to submit ExecuteRequest, I need to get a list of TypedValue

So, you're saying that you cannot execute a prepared query with an array substituted in? I'd like to make sure I understand exactly what you're trying to do before digging into this. The more clarification you can provide on what exactly you are trying to do would be extremely helpful.

I can't really use the protobuf interface, because it's using the beta version of proto3, which would make my code very hard to install.

Sorry to hear that. There are published tarballs for the protobuf releases, but I guess you're lamenting packages provided by your operating system? Even so, you could build the python libs and include them in your python module, no? It seems like something that could be solved in the packaging of your phoenixdb project. Am I misrepresenting this problem? Protobuf is the way of the future as far as Avatica is concerned. I'd like to make sure the path forward isn't unnecessarily difficult.

Let's say I have a prepared statement, created like this:

{
  "request": "prepare",
  "connectionId": "...",
  "sql": "UPSERT INTO foo (id, array_column) VALUES (?, ?)",
  "maxRowCount": 0
}

Now I need to execute the statement like this:

{
  "request": "execute",
  "statementHandle": { ... },
  "parameterValues": [
    {"type": "INTEGER", "value": "1"},
    {"type": "???", "value": ???}
  ],
  "maxRowCount": 0
}

I don't know what to put instead of ??? for the array parameter to work. ColumnMetaData.Rep does have an ARRAY type, but it's either not used in TypedValue or I'm submitting the data in the wrong format. Initially I expected "type": "ARRAY", "value": [1,2,3] to work, but it doesn't.

Thanks for writing this up Lukas Lalinsky. This is a great example. I see what you mean now.

On the "query results" side of things, we have Frame which which contains a List<Object> of rows. For an array, the element in the rows would be a List itself, which triggers it. I'm guessing that it was just missing on this side.

Let me start by writing some tests to work through this. Thanks again for the bug report.

Ok, this is most assuredly missing the implementation. Calling setArray on the AvaticaPrepareStatement, ends up calling AvaticaSite#setArray:

  public void setArray(Array x) {
  }

I need to give this some thought about how to implement, but I'm a bit scared about trying to tackle this so close to the 1.6.0 release. I may punt this to 1.7.0 since it would require a bit of code-change and testing.

Josh Elser - is this one still on your radar? Over in Phoenix/Calcite integration land, this one would help a lot.

Yes, it is James Taylor. Sorry I haven't gotten to finishing it yet..

Just wanted to drop a line: I've had some time to work on this yesterday and today. I think I'm close.

/me grins

0: jdbc:avatica:remote:url=http://hw10447.loc> select * from arr;
+-------+------------+-------------------------+
| name  |  int_arr   |        schedule         |
+-------+------------+-------------------------+
| josh  | [1, 2, 3]  | [[a, b, c], [b, c, d]]  |
+-------+------------+-------------------------+

GitHub user joshelser opened a pull request:

https://github.com/apache/calcite-avatica/pull/2

CALCITE-1050 Array support

Tested against HSQLDB, Postgres, and Apache Phoenix

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/joshelser/calcite-avatica 1050-arrays

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/calcite-avatica/pull/2.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2

commit 34d1016ea23492abb1e879cd3f560741e69338bc
Author: Josh Elser <elserj@apache.org>
Date: 2016-01-12T21:40:18Z

CALCITE-1050 Array support for Avatica

As best as possible, works around the limitations of JDBC's
Array class to handle arbitrarily nested Arrays. Nested-array
support differs from DB to DB, so functionality is primarily
driven from structure, rather than metadata.

commit 1097142106478605741dff77063a59bbd5f50909
Author: Josh Elser <elserj@apache.org>
Date: 2017-03-30T20:34:33Z

Further cleanup

• Remove dead code
• Remove Compiler warnings
• Clarify some comments

https://github.com/apache/calcite-avatica/pull/2 includes some basic array support. I have tested this one against HSQLDB (functionally and via unit tests), Postgresql (notably for its support of multi-dimensional array support), and Apache Phoenix (as there was interest/need here).

James Taylor, Rajeshbabu Chintaguntla, can either of you put this through some paces with the Calcite-Phoenix integration? I've just done some rudimentary tests with Phoenix so far.

Julian Hyde, another good one if you have some time to devote to a review

Josh Elser Thank you very much for your work. Sure will take a look at it.

Maryann Xue - would you have a few spare cycles to review this patch?

If it doesn't go without saying, do let me know if there's anything I can do to help make this review from the Phoenix side easier.

I've just done simple testing via the Phoenix Query Server, but I could push a SNAPSHOT build of Avatica and Calcite up to repository.a.o which you could switch the phoenix-calcite branch to use.

Argh, just realized that I broke some stuff in Calcite. Looking into those...

Argh, just realized that I broke some stuff in Calcite. Looking into those...

Pushed an update to the PR which at least gets a Calcite mvn verify passing again.

Thank you very much for doing the Array support, Josh Elser! One important thing though, can you please add the interface level support as well, such as AvaticaConnection.createArrayOf?

One important thing though, can you please add the interface level support as well, such as AvaticaConnection.createArrayOf?

Sure thing, I can look at implementing that method! Any others on Connection?

What about the other methods on Array? Only some of the getArray(..) and getResultSet(..) methods are implemented currently. It's been enough to at least run against sqlline with a couple of databases. Just curious if you know of any specifically that are higher priority to implement (also not sure how much work each will be).

Not sure, but I think other than createArrayOf, setArray, and getArray are enough for most test cases in Phoenix. And they have already been implemented in the PR, thanks! One thing is that ResultSet.getString is also used on array fields in Phoenix tests to get the "toString" value of an array, I am not sure if it's SQL standard or if Avatica supports it that way. James Taylor, do you have any comment on this point?

Another problem I found by running the Phoenix array tests was that literal string array values were taken as CHAR ARRAY type and could not be inserted into VARCHAR ARRAY columns. I think this is a Calcite issue and I'll open a separate JIRA.

Thanks so much for this, Josh Elser!

Review comments:

• In jdbcToSerial it doesn't look as if it handles null Timestamp values.
• In AvaticaUtils, the getNonPrimitiveRep and getSerializedRep methods can be moved to Rep, and maybe rationalized a bit.

Not sure, but I think other than createArrayOf, setArray, and getArray are enough for most test cases in Phoenix. And they have already been implemented in the PR, thanks!

Excellent . Thanks for digging in, Maryann.

One thing is that ResultSet.getString is also used on array fields in Phoenix tests to get the "toString" value of an array, I am not sure if it's SQL standard or if Avatica supports it that way

Yup, this is pretty normal. Essentially, just the same as you can get a String representation of a value of a column in the current row of a ResultSet, you can treat a ResultSet generated from an Array in the same manner. Makes the user-facing parsing a bit more consumable (thankfully, as the Array API leaves a bit to be desired).

Another problem I found by running the Phoenix array tests was that literal string array values were taken as CHAR ARRAY type and could not be inserted into VARCHAR ARRAY columns. I think this is a Calcite issue and I'll open a separate JIRA.

Ok, let me know if you find otherwise, or have a test case for it.

Review comments:

Thanks for looking, Julian. I need to write some test cases to catch the Calcite failures I fixed last night in Avatica (to prevent the "surprise, calcite's busted" situation). I'll look at these two suggestions as well!

Just pushed an update to the existing PR which, I believe, addresses the comments from Julian and Maryann (thanks so much, you two).

Along the way, I also found CALCITE-1756, and fixed an existing bug with Array.getArray(long, int). I'd like to get this committed (as it's rather large) and then go into bug-fix mode with any subsequent issues.

Looks like I broke some more stuff in Calcite due to interface changes . Will see if I can avoid forcing a Calcite code-change.

Will see if I can avoid forcing a Calcite code-change.

While some of the API/interface changes I made in Avatica were simplifications (unused arguments in method signatures), I reverted the removals to make the upgrade to avatica 1.10.0 less painful in Calcite.

There is one failing test out of the box (JdbcTest.testPreparedStatement) which expects a very specific exception message. Due to some unnecessary method-calls (as alluded to in the above sentence), an Exception is triggered in a different part of the code, and generates a different error message. I think we should just change the test itself in this case.

I've been poking at ArrayIT in Phoenix and it seems like Phoenix was making some assumptions that it would get a PhoenixArray as the bound parameter value whereas, with these changes, it's actually get an Java array (e.g. Object[]). This is a result of PhoenixCalciteConnection calling toJdbc(..) on TypedValue. Per the Javadoc (that predates me), the JDBC form of an ARRAY would be an Object[]. Thus, the RuntimeContextImpl generates Object[]'s for the Array's, and Phoenix later tries to cast the Object[] to an Array (granted, a PhoenixArray which wouldn't work at all – but an Array would also fail). So, I'm not sure there's much more we can glean from Phoenix to assert correctness here (requires more work in Phoenix).

But, before I push this in, I am left wondering: Julian Hyde, is the JDBC form of an Array an Object[] correct? (for context: https://github.com/joshelser/calcite-avatica/blob/1050-arrays/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java#L112-L113). The method signature of PreparedStatement.setArray(int, Array) as opposed to PreparedStatement.setArray(int, Object) (or some form of a Java array, Object or primitive).

A JDBC array is represented by the java.sql.Array interface (https://docs.oracle.com/javase/7/docs/api/java/sql/Array.html). It's entirely likely that Phoenix unit tests assume the backing implementation is a PhoenixArray.

Yeah, agreed on the Array interface portion, James. I think I saw the code I was stepping through was in some PData type classes, but I'd have to pull it up again.

My hunch here is that I've fallen into a typo in the docs, but maybe there's a reason this was documented as such . Granted, even if this was the case, we'd still be getting the ClassCastExceptions (trying to cast a Avatica's ArrayImpl into PhoenixArray, but that's for another day).

As far as I can tell, a JDBC driver is safe to assume that every Array it sees is one of its own. If a user creates class MyArrayImpl implements Array and passes it to PreparedStatement.setArray they will get an error.

As a user, if I want to create an array, I must call Connection.createArrayOf.

As far as I can tell, a JDBC driver is safe to assume that every Array it sees is one of its own. If a user creates class MyArrayImpl implements Array and passes it to PreparedStatement.setArray they will get an error.

As a user, if I want to create an array, I must call Connection.createArrayOf.

That makes sense to me. Something else to fix up down in Phoenix land.

Should I make the leap to say that you agree with me that TypedValue is doing the wrong thing (by not creating an Array object for the JDBC form)?

I don't think TypedValue is wrong. Its job is to send values over the wire, right? Array values aren't serializable, because they contain a connection. So I think you should serialize the values in the array (or map or struct or multiset) in something close to their native values then wrap them in your implementation of Array at the other end.

I see you've added logic to AvaticaConnection.createArray to map type names to AvaticaType instances. At some point (not yet) we will want to convert that to a parser and caching map that can handle complex types and parameterized types such as "DECIMAL(10, 4)".

I don't think TypedValue is wrong. Its job is to send values over the wire, right? Array values aren't serializable, because they contain a connection. So I think you should serialize the values in the array (or map or struct or multiset) in something close to their native values then wrap them in your implementation of Array at the other end.

Yes, I agree with you on the serial/local forms. I'm more trying to reconcile what the docs say the JDBC form is used for. Here's a snippet of code which I'm trying to use to reason about this:

  @Test public void testSetJavaArray() throws Exception {
    try (Connection conn = DriverManager.getConnection(url)) {
      Integer[] data = new Integer[] {1, 2, 3, 4, 5};
      final Array array = conn.createArrayOf("INTEGER", data);
      final TypedValue arrayTypedValue =
          TypedValue.ofJdbc(Rep.ARRAY, array, Unsafe.localCalendar());
      final String tableName = "SET_JAVA_ARRAY";
      try (Statement stmt = conn.createStatement()) {
        assertFalse(stmt.execute("DROP TABLE IF EXISTS "  + tableName));
        assertFalse(stmt.execute("CREATE TABLE " + tableName + "(pk integer not null primary key,"
            + " numbers integer array)"));
      }
      try (PreparedStatement pstmt = conn.prepareStatement("INSERT INTO " + tableName
          + " values(?,?)")) {
        pstmt.setInt(1, 1);
        pstmt.setArray(2, (Array) arrayTypedValue.toJdbc(Unsafe.localCalendar()));
        assertEquals(1, pstmt.executeUpdate());
      }
  }

Given the javadoc, I'd expect the above to work. For a TypedValue, I should get an Object which is suitable to be set as an Array on the PreparedStatement. However, the above throws a ClassCastException (trying to cast Object[] to Array).

Given the javadoc, I'd expect the above to work. For a TypedValue, I should get an Object which is suitable to be set as an Array on the PreparedStatement. However, the above throws a ClassCastException (trying to cast Object[] to Array).

Alright, I have convinced myself that the current javadoc on TypedValue is wrong. The reason we have not noticed this in the past is that HSQLDB will "coerce" Object[] (non-primitive type arrays) and List<Object> into columns of type ARRAY. For correctness: TypedValue.toJdbc(Calendar) should return an Array, not a List. Databases that happened to still work are just playing "nice".

Github user asfgit closed the pull request at:

https://github.com/apache/calcite-avatica/pull/2

Thanks for all of the reviews/feedback, folks.

Fixed in https://git-wip-us.apache.org/repos/asf?p=calcite-avatica.git;a=commit;h=dd65a2b18b8c35cfccf1c47b6be87ea7db3ad658

Resolved in release avatica-1.10.0 (2017-05-30).