xtdb

seancorfield 2024-11-24T07:08:48.562349Z

I've added the new xtdb-jdbc module to next.jdbc's testing setup and I'm updating the tests to work with a required _id column (instead of an auto-generated id column), and I'm slowly making progress:

Ran 360 tests containing 3651 assertions.
124 failures, 9 errors.
which is a lot better than where I started a couple of hours ago...

refset 2024-11-25T11:57:32.159749Z

This is what it has takenThanks again for your efforts and for sharing that. I guess an update-able CREATE VIEW with an alias might have helped? πŸ€”

seancorfield 2024-11-25T17:22:18.114279Z

Not sure what that means or what it would add? The main changes in the test suite are: β€’ _id is the primary key in all tables rather than (by convention) id or some other column β€’ insert ... returning * doesn't return anything useful -- all other databases can return the newly-inserted row, or at least the newly-generated primary key β€’ since .getTableName() doesn't return the table name associated with a column in a query result set, the "traditional" qualified column names -- :table/column -- are not possible like they are with other databases For the latter, Oracle also behaves like XTDB in that respect -- but I don't test against it -- and MS SQL Server requires specific parameters set on the PreparedStatement for it to be able to return table names (which I provide in the tests that matter). PostgreSQL provides those table names, but calling .getTableName() causes the driver to issue a metadata query behind the scenes to fetch that data. For the most part, that third bullet was already handled in most of the test suite, via the column function which also handled the natural upper/lower case variations across databases, but the first two caused extensive changes.

refset 2024-11-25T17:39:40.901869Z

ah sorry, I meant something like: CREATE VIEW fruit AS SELECT _id AS id, * FROM actual_fruit; then you could stick with your convention-based approach: select * from fruit order by id

seancorfield 2024-11-25T17:40:53.826279Z

Ah, interesting. I haven't worked with views in databases for a very long time...

πŸ™‚ 1
refset 2024-11-25T17:40:57.543699Z

> .getTableName() doesn't return the table name associated with a column in a query result set thanks, this sounds like something we might want to add - not sure I've seen that mentioned before

seancorfield 2024-11-25T17:41:08.762459Z

I've mentioned it several times πŸ™‚

seancorfield 2024-11-25T17:41:42.657899Z

(in the context of qualified column names and the general HoneySQL/`next.jdbc` approach to :table/column and not having / in column names πŸ™‚ )

refset 2024-11-25T17:42:01.791579Z

ahh, apologies, my JDBC experience is very basic and my eyes glaze over πŸ˜…

seancorfield 2024-11-25T17:42:27.572269Z

Heh... like me and Datalog, I suspect...

seancorfield 2024-11-25T17:44:08.767229Z

I saw your comment about / round-tripping in PG, BTW. It really isn't something I want to support in HoneySQL or next.jdbc (and no PG users have ever asked for it) -- and in fact I just ripped out all the experimental support for it this weekend, after confirming that XTDB doesn't allow / in general... and I was pleased to see none of the examples in the docs seemed to use this any more...

πŸ™‚ 1
refset 2024-11-25T17:46:52.803819Z

completely understand yep, I just want to make sure all perspectives are considered - e.g. the (potential, hypothetical) need for 'expanded' URI keys in JSON-LD handling

refset 2024-11-25T17:46:59.516119Z

just opened https://github.com/xtdb/xtdb/issues/3906 πŸ™ (thanks for your persistence mentioning it!)

seancorfield 2024-11-25T17:54:42.536719Z

When I get my energy back after all that XTDB-related work over the weekend on HoneySQL and next.jdbc, I'll go through the changes in the test suite and see if there are any other "obvious" issues I should create for XTDB... I did create one for the "unexpected error on tx submit" I encountered.

πŸ™ 1
jarohen 2024-11-25T19:02:19.682819Z

thanks @seancorfield - much appreciated, as always πŸ™

seancorfield 2024-11-24T08:51:45.963209Z

Down to this now, so I'll pick it up again tomorrow:

Ran 360 tests containing 3647 assertions.
78 failures, 1 errors.

jarohen 2024-11-24T10:24:43.421639Z

great news, thanks @seancorfield ☺️

seancorfield 2024-11-24T17:02:55.612879Z

12 failures, 1 errors.

seancorfield 2024-11-24T17:25:57.834979Z

Success!

Ran 360 tests containing 3637 assertions.
0 failures, 0 errors.

πŸŽ‰ 4
seancorfield 2024-11-24T17:32:57.571299Z

This is what it has taken to get to no failures: https://github.com/seancorfield/next-jdbc/compare/develop...xtdb-testing?w=1

seancorfield 2024-11-24T07:21:06.146019Z

Q: Is .setMaxRows implemented on Statement for XTDB? I'm setting this to 2 but getting back more than two rows so I'm guessing it isn't?

refset 2024-11-25T12:00:19.370689Z

> I do recall seeing a TODO for it in the codebase https://github.com/xtdb/xtdb/blob/52ec4d4a6a6790de43814577f370fdce028ad1f0/core/src/main/clojure/xtdb/pgwire.clj#L1540?

πŸ‘ 1
jarohen 2024-11-24T10:25:06.438079Z

not yet, no, although I do recall seeing a TODO for it in the codebase πŸ™Š

seancorfield 2024-11-24T15:45:32.633559Z

Should I create a GH issue for it, for tracking purposes? Once I'm done with this next.jdbc testing pass, I may have several "issues" to raise πŸ™‚

seancorfield 2024-11-24T16:16:24.372129Z

As I'm working on getting all the next.jdbc tests passing on XTDB (or at least disabling tests that cannot work... yet), I'm finding that if I run my failing test suite multiple times, after a few runs I get this error:

Execution error (PSQLException) at org.postgresql.core.v3.QueryExecutorImpl/receiveErrorResponse (QueryExecutorImpl.java:2733).
ERROR: unexpected error on tx submit (report as a bug)
(stacktrace in 🧡 )

seancorfield 2024-12-03T03:30:20.152419Z

I have a PR open against next.jdbc to enable automated XTDB testing going forward -- https://github.com/seancorfield/next-jdbc/pull/290 (so you can also see all the changes required to get the test to pass with XTDB -- mostly that _id is a fixed primary key (instead of the convention of id -- not all DBs allow columns to begin with _!) and that insert doesn't (can't) return the inserted PK. I won't merge it until #3894 is addressed, but at least I can easily test any future changes to next.jdbc against XTDB automatically!

2
☺️ 1
seancorfield 2024-11-24T16:18:37.878799Z

It happens in different places on different runs, but once XTDB gets into this state, the only solution is to shutdown the Docker instance and restart it (and sometimes I need to rm the container and recreate it).

seancorfield 2024-11-24T16:20:08.286439Z

This branch has the current state of play, if you want to try for yourselves: https://github.com/seancorfield/next-jdbc/tree/xtdb-testing Clone the repo, checkout that branch, run docker compose up, and then run NEXT_JDBC_TEST_XTDB=yes clojure -X:test a few times.

seancorfield 2024-11-24T16:39:29.130979Z

And here's the log output from the XTDB Docker instance when it fails like that:

seancorfield 2024-11-24T17:33:51.765529Z

Note: even having reached 0 failures, 0 errors, if you run the test command several times, you'll still hit this "unexpected error"...

jarohen 2024-11-24T21:21:33.942799Z

thanks @seancorfield - yep, we shouldn't be trying to do that. if you get there before me, could you raise an issue for this one?

seancorfield 2024-11-24T21:26:46.216879Z

I used Slack to create an issue from this Slack thread. Not sure how clear/useful that will be but I think it links back to this thread...