Excellent!

Unless I did something silly, this should be just Oracle failures now. Need to investigate those more.

About to head out for ~2 weeks of PTO...

About test failures:

1. MariaDB - one test fails (LockedRowsTests#testFindSkipLocked). It does not fail locally if I just run that test or run all the tests in that class or run all the tests in that package or ... makes it hard to debug, but I'll gwt back to it after PTO
2. Oracle - I have not even started digging into these. I'm 100% certain it is because of the way AbstractSqlAstTranslator has LOTS of code specific to Oracle. Handling of for-update was definitely part of that.

Some things I'd still like accomplish here...

1. The main one is to move rendering of the lock fragments from Dialect proper into the LockingSupport impls.
2. A related task is to make LockingSupport.Metadata sensitive to write/read locking. I think tjis will be needed for (1).
3. Integrate support for LockTimeoutType#CONNECTION into the acquisition of locks when performing find/lock/refresh operations. I've implemented ConnectionLockTimeoutStrategy for quite a few dialects already, which handles the actual setting/resetting.
4. Adjust design of JdbcOperation to simply be a concept of something that needs to be performed via JDBC. Currently it implies a single statement. Ultimately I want this to be open-ended and mean 1-or-more statements. This would allow grouping all the SQL needed for locking into a single JdbcOperation and combine ConnectionLockTimeoutStrategy, follow-on locking as well as the main query.

WRT (2), currently I have:

public interface LockingSupport {
	Metadata getMetadata();

	...

	interface Metadata {
		LockTimeoutType getLockTimeoutType(Timeout timeout);
		RowLockStrategy getReadRowLockStrategy();
		RowLockStrategy getWriteRowLockStrategy();
		...
	}
}

public class Dialect {
	...
	LockingSupport getLockingSupport();
}

Thinking instead of something like:

public interface LockingSupport {
	Metadata getMetadata();

	...

	interface Metadata {
		LockTimeoutType getLockTimeoutType(Timeout timeout);
		RowLockStrategy getRowLockStrategy();
		...
	}
}

public class Dialect {
	...
	LockingSupport getWriteLockingSupport();
	LockingSupport getReadLockingSupport();
}

Let's see if the MariaDB run still fails. The test failure there makes no sense and I cannot reproduce it locally. It is testing SKIP LOCKED support, which MariaDB supports since 10.3 or 10.6 (I've seen conflicting info online). Locally, at least, the image is:

	Database driver: org.mariadb.jdbc.Driver
	Database version: 11.7.1

Let's see if the MariaDB run still fails.

LockedRowsTests#testFindSkipLocked does still fail with MariaDB on CI. Still passes locally. So... 🤷‍♂️

I skipped it for MariaDB.

Thinking instead of something like:

public interface LockingSupport {
	Metadata getMetadata();

	...

	interface Metadata {
		LockTimeoutType getLockTimeoutType(Timeout timeout);
		RowLockStrategy getRowLockStrategy();
		...
	}
}

public class Dialect {
	...
	LockingSupport getWriteLockingSupport();
	LockingSupport getReadLockingSupport();
}

The next steps should be to move actual handling of applying locking from the Dialect into this support contract. With that in mind, I'm thinking something like this:

public interface LockingSupport {
	/**
	 * Used as part of {@linkplain org.hibernate.sql.ast.SqlAstTranslator SQL AST translation}.
	 */
	LockingClauseStrategy getLockingClauseStrategy(QuerySpec querySpec, LockOptions lockOptions);

	/**
	 * Used with {@linkplain org.hibernate.Session#lock Session-based locking}
	 */
	LockingStrategy getLockingStrategy(EntityPersister lockable, LockMode lockMode, Locking.Scope lockScope);

	/**
	 * Used to apply locking to simple String-based SQL building, such as
	 * {@linkplain org.hibernate.Session#createNativeQuery "native" queries}.
	 */
	String applyLocksToSql(
			String sql,
			LockOptions aliasedLockOptions,
			Map<String, String[]> keyColumnNames);

	Metadata getReadLockMetadata();
	Metadata getWriteLockMetadata();

	OuterJoinLockingType getOuterJoinLockingType();
	ConnectionLockTimeoutStrategy getConnectionLockTimeoutStrategy();

	interface Metadata {
		default PessimisticLockStyle getPessimisticLockStyle() {
			return PessimisticLockStyle.CLAUSE;
		}

		default LockTimeoutType getLockTimeoutType(Timeout timeout) {
			// matches legacy definition from Dialect where only "wait forever" was supported
			if ( timeout.milliseconds() == Timeouts.WAIT_FOREVER_MILLI ) {
				return LockTimeoutType.QUERY;
			}
			return LockTimeoutType.NONE;
		}

		default RowLockStrategy getRowLockStrategy() {
			// by default, we report no support
			return RowLockStrategy.NONE;
		}
	}
}

As we cannot immediately remove methods such as Dialect#getForUpdateString, Dialect#getReadLockString, Dialect#getWriteLockString, ... we will need to (temporarily?) expose similar methods here as well.

Marking as ready for review, but keep in mind that this is just the first steps. I simply want to get this incorporated upstream asap to not have to deal with these painful rebases from upstream.