fix ListMigrations query for YDB

[asmyasnikov]

Fix for #684

Some postgresql compatibility was broken. Thats why a query with --!syntax_pg not worked on trunk version of YDB
Rewrite of query for ListMigrations can helps for YDB since 23.3 and to trunk version

[mfridman]

The ordering here should either be id or timestamp, but not version.

In the future, this may change, but for now, this query should return a list of migrations in the order they were applied.

[asmyasnikov]

Is version_id not ordered by migrations?

[asmyasnikov]

Clickhouse have the same query
https://github.com/pressly/goose/blob/master/internal/dialect/dialectquery/clickhouse.go#L37

[mfridman]

By default, yes. But, it's not guaranteed because goose supports out-of-order migrations.

https://pressly.github.io/goose/blog/2021/out-of-order-migrations/

Outside the scope of this PR, but we're considering dropping this behaviour so it always operates on the version id (mainly important for down migrations). For context:

#523 and #402

[mfridman]

ClickHouse is a bit of a special case, but I also consider that a bug.

The only time this matters is if you're opting into out-of-order migrations and applying down migrations. If you have migrations that were applied <1s apart then we cannot guarantee deterministic ordering.

A bit more context on this here:

#520 (comment)

Most users do not use out-of-order and so this is a non-issue for them.

[asmyasnikov]

I suggest approve this PR.
And after fix issue ydb-platform/ydb#1350 (maybe 24.1 - 24Q1-24Q2) I will contribute another PR with ordering by tstamp

[mfridman]

I suppose that's the least-worst option, better to have an edge case as opposed to a non-functioning implementation.

Again, this is really only an issue for down migrations when allowing out-of-order.

[asmyasnikov]

Alternate way - re-write query to

SELECT version_id, is_applied FROM (
    SELECT version_id, is_applied, tstamp 
    FROM goose_db_version ORDER BY tstamp DESC
)

This is working query, but it has no garanties for ordering in bounded SELECT
The order is preserved right now, but it may break down in the future

[mfridman]

@asmyasnikov Let's update the test to capture the expected behaviour. And we'll go ahead and merge as-is.

[asmyasnikov]

What do you think about common change - #687 ?

[mfridman]

What do you think about common change - #687 ?

I'd prefer not to make a sweeping change without carefully evaluating the impact. There's a bit of nuance to some of this and we should keep this PR scoped to fixing YDB. I'd like to put a bit more thought into going the opposite way and removing the listing behaviour entirely in favor of version ordering only.

[asmyasnikov]

@mfridman I released ydb-go-sdk with discard column (tstamp) on driver-side. This is not good solution, but it is work

[mfridman]

@mfridman I released ydb-go-sdk with discard column (tstamp) on driver-side. This is not good solution, but it is work

As mentioned before, I have no objection to using version_id for ydb (now that I have a bit more understanding about the driver), I'll leave the implementation to your discretion and if this solution isn't good, then let's do version_id.