Fix trino-cli to display plans for EXPLAIN ANALYZE

[homar]

Description

Previously trino-cli didn't display plans for EXPLAIN ANALYZe

Is this change a fix, improvement, new feature, refactoring, or other? a fix Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific) a client library How would you describe this change to a non-technical end user or system administrator? Gives user proper output for EXPLAIN ANALYZe

Related issues, pull requests, and links

Documentation

(x) No documentation is needed. ( ) Sufficient documentation is included in this PR. ( ) Documentation PR is available with #prnumber. ( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required. (x) Release notes entries required with the following suggested text:

# cli
* trino-cli displays query plan for EXPLAIN ANALYZE queries ({issue}`issuenumber`)

[electrum]

Actually, I think we still want to send update type and count, since an EXPLAIN ANALYZE can still have an update count, and the client might want to see or access it.

We can remove the code that produces the fake result in the server. We should verify that all of our clients support this properly.

For the JDBC driver, it needs to properly implement Statement.getMoreResults() so that it returns the result set rows if they exist (i.e. if the server returns a columns field in the query results).

[homar]

Actually, I think we still want to send update type and count, since an EXPLAIN ANALYZE can still have an update count, and the client might want to see or access it.

We can remove the code that produces the fake result in the server. We should verify that all of our clients support this properly.

For the JDBC driver, it needs to properly implement Statement.getMoreResults() so that it returns the result set rows if they exist (i.e. if the server returns a columns field in the query results).

@electrum Currently we don't send updateCount, it is set to null for EXPLAIN ANALYZE, we only send updateType. Should I try to fix sending updateCount for EXPLAIN ANALYZE? If so, assuming updateCound and updateType are present how should I fix this if not by using hacky solution like this one ?

[electrum]

EXPLAIN ANALYZE is special in that it runs the statement. Its update type should be the update type of the underlying statement (if present). It should also return the update count (if present).

The reason we have the "discard results" hack in the CLI and JDBC driver is due to the server adding a fake result set for update statements. If we remove that on the server side, then there doesn't need to be any special handling in the clients.

With this change, the JDBC driver will need to support the multiple result sets: the update count and the explain analyze output. JDBC explicitly supports multiple results as explained in the Javadoc. We should have the update count be returned first (the first result set) since that will work correctly with older servers that return the fake result set.

[homar]

@electrum Could you please take a look now?

[homar]

Pushing a version without all of the changes to make sure changes so far are correct

[findepi]

will merge tomorrow

[martint]

What was actually fixed with this PR?

The current version (397) shows query plans for explain analyze:

trino> select version();
 _col0
-------
 397
(1 row)

Query 20220925_231758_00003_r6mch, FINISHED, 1 node
Splits: 1 total, 1 done (100.00%)
0.06 [0 rows, 0B] [0 rows/s, 0B/s]

trino> EXPLAIN ANALYZE select * from tpch.tiny.orders;
                                                                                              Query Plan
-------------------------------------------------------------------------------------------------------------------------------------------
 Fragment 1 [tpch:orders:15000]
     CPU: 194.57ms, Scheduled: 194.66ms, Blocked 0.00ns (Input: 0.00ns, Output: 0.00ns), Input: 15000 rows (1.86MB); per task: avg.: 15000.
     Output layout: [orderkey, custkey, orderstatus, totalprice, orderdate, orderpriority, clerk, shippriority, comment]
     Output partitioning: SINGLE []
     TableScan[table = tpch:tiny:orders]
         Layout: [orderkey:bigint, custkey:bigint, orderstatus:varchar(1), totalprice:double, orderdate:date, orderpriority:varchar(15), cl
         Estimates: {rows: 15000 (1.52MB), cpu: 1.52M, memory: 0B, network: 0B}
         CPU: 194.00ms (100.00%), Scheduled: 194.00ms (100.00%), Blocked: 0.00ns (?%), Output: 15000 rows (1.86MB)
         Input avg.: 3750.00 rows, Input std.dev.: 0.00%
         clerk := tpch:clerk
         orderkey := tpch:orderkey
         orderstatus := tpch:orderstatus
             :: [[F], [O], [P]]
         custkey := tpch:custkey
         totalprice := tpch:totalprice
         comment := tpch:comment
         orderdate := tpch:orderdate
         orderpriority := tpch:orderpriority
         shippriority := tpch:shippriority


(1 row)

[findepi]

What was actually fixed with this PR?

EXPLAIN ANALYZE for update statements. Will fix the PR title

[findepi]

for posterity, i've updated https://github.com/trinodb/trino/issues/14245#issuecomment-1259174004 too thanks for catching this