Investigate whether any rejected refactor decision should be superseded #282

The owner rejected six refactor ideas on purpose during #194. This issue is a place to record evidence for or against revisiting each one. It does not authorize a change. Each item needs its own probe and its own decision before any code moves.

Acceptance for this issue is a written recommendation per row. No row changes code until the owner accepts it.

Decision record for the six rejected refactors

Read #194, the pass PRs #216-#236, and the current tree at 3760d39. No code changed. No existing assertion changed. Verdicts: 4 KEEP, 1 SUPERSEDE, 1 NEEDS OWNER DECISION.

1. Split the unsupported-operation blocks

Original rejection. Owner magnusp, #194: "No line savings, and grouping tempts normalizing UnsupportedOperationException against SQLFeatureNotSupportedException."

Code and pins. The unsupported-operation surface has 97 throws in LibsqlResultSet. 94 use "not implemented by SQLite JDBC driver". The other three are "Type maps are not supported", "not supported by sqlite", and the declared-type message at line 138. The unsupported update methods span lines 841-1060. LibsqlDatabaseMetaData has 7 unsupported-operation throws, LibsqlStatement 6, LibsqlConnection 8, LibsqlPreparedStatement 2, LibsqlDriver 1, and the LOB wrappers 14. LibsqlConnection.java:812 and :815 throw bare UnsupportedOperationException for createArrayOf and createStruct. Every other unsupported operation throws SQLFeatureNotSupportedException. The pins are LibsqlResultSetTest, Jdbc4AppendixBProfileTest#everyUnsupportedDeclaredTypeThrowsFeatureNotSupported, and the xerial conformance suite.

Smallest probe. Count the throws, the distinct messages, and the exception types. A base-class split moves the 97 lines and leaves one inherited method per operation. The public class still exposes every method, and the reader now has two files to trace. Grouping the two bare UnsupportedOperationException sites with the sites that throw SQLFeatureNotSupportedException would change the exception type, which is contract.

Gain. None measured. The change moves lines and adds indirection. KEEP.

2. Merge valueAtCursor with valueForMetadata

Original rejection. Owner magnusp, #194: "Different NULL semantics."

Code and pins. LibsqlResultSet.java:110-113 returns Value.ofNull() when there is no current row and does not bounds-check. LibsqlResultSet.java:143-154 returns Java null when the row index is past the end, when the values list is null or short, or when the column is out of range. The pins are LibsqlResultSetTest, LibsqlResultSetNavigationTest, and LibsqlResultSetMetaDataTest.

Smallest probe. A six-state truth table:

A merged helper returning Java null plus a wrapper saves at most a few lines and adds a translation a reader must hold. Open issue #276 says "Leave valueAtCursor and valueForMetadata in LibsqlResultSet." The change would conflict with that scope.

Gain. At most a few lines, higher reader load. KEEP.

3. Rewrite the REAL formatter or the type if-chains

Original rejection. Owner magnusp, #194: "The quirks are the contract with xerial."

Code and pins. XerialColumnType.java was deleted in e41cac1 (#247), and JdbcTypeMapping now resolves declared types through a 36-entry registry and a storage-class switch. LibsqlResultSet still has type if-chains in the getObject fast path, objectValue, and getObject(int, Class). Open #276 says "Keep the getObject fast path and the type if-chains byte for byte." SqliteRealText.java is 272 lines, ported from SQLite's %!.17g. LibsqlResultSetRealTextTest has 5 test methods, two parameterized with 11 cases each, and covers the canonical fixed and scientific forms, BigDecimal scale, and repeated reads. The negative-zero path is handled in code and was probed in #216, but no test pins it.

Smallest probe. git log --diff-filter=D shows the XerialColumnType deletion. grep shows the registry and the remaining getObject if-chains. The required evidence was a byte-for-byte parity suite over the formatter and the type matrix plus a measured readability gain. The formatter suite exists. The type matrix is covered by Jdbc4AppendixBTypeMappingIT and Jdbc4AppendixBReadTest. No readability gain is measured for a rewrite, and a rewrite of a bit-exact port carries parity risk. #276 already pins the result-set if-chains byte for byte, so a rewrite would conflict with an open scope.

Gain. None measured. The extracted if-chain half is gone through #247, and the result-set if-chains are pinned by #276. KEEP.

4. Unify Locale.ENGLISH and Locale.ROOT upper-casing

Original rejection. Owner magnusp, #194: "No reader-load gain." The evidence bar in #282 was "a locale-sensitive test that shows the two are interchangeable for every call site."

Code and pins. The three Locale.ENGLISH sites are ColumnMetadata.java:123, DeclaredType.java:44, and ForeignKeyMetadata.java:131. The other 19 case-folding sites use Locale.ROOT, including the two fully qualified java.util.Locale.ROOT sites in LibsqlConnection. The pins are LibsqlDatabaseMetaDataTest#getPrimaryKeysReadsAConstraintNameGluedToTheKeywords, LibsqlResultSetMetaDataTest#preservesWhitespaceInDeclaredTypeNames, and the foreign key rule tests.

Smallest probe. A scratch Java program compared String.toUpperCase and String.toLowerCase under both locales for every Unicode code point from U+0000 to U+10FFFF, excluding surrogates, and for the declared type names, foreign key rules, SQLite error codes and messages, and identifiers that reach the call sites. The result was 0 upper differences and 0 lower differences. The two locales are interchangeable for every input the driver can see.

Gain. One locale policy. A reader no longer has to decide whether Locale.ENGLISH at three sites means something different from Locale.ROOT at nineteen. The probe proves no result changes. The production diff is three tokens. Parity is preserved. Confidence is medium because the gain is consistency, not a line reduction.

New issue. #283. It records the superseded decision, the probe, the three call sites, the behavior pin, and the acceptance criteria. Implementation. #285. Follow-up. #286.

5. Consolidate the two constraint-name parsers

Original rejection. Owner magnusp, #194: "Both are pinned byte for byte, and the swap carries behavior risk with no structural win."

Code and pins. ForeignKeyMetadata.java:24-29 holds NAMED_PRIMARY_KEY and NAMED_FOREIGN_KEY. The primary key pattern allows \s* after CONSTRAINT; the foreign key pattern requires \s+. The pins are LibsqlDatabaseMetaDataTest lines 403-475 for the FK and PK names and lines 547-554 for the glued primary key forms.

Smallest probe. A scratch Java program compared the two original patterns with three shared-parser candidates over the pinned statements plus glued, bracketed, single-quoted, double-quoted, lower-case, no-space, and multi-FK cases. The difference column is the meaningful one, because a null-equals-null result counts as a match.

A shared parser that keeps both pinned grammars needs a whitespace parameter. That is a factory, not a consolidation, and it saves no lines. A shared parser that normalizes the whitespace widens the foreign key grammar to forms no pin covers.

Gain. None. The original rejection stands. KEEP.

6. Remove the unused public LibsqlResultSetMetaData(Statement, StmtResult) constructor

Original rejection. Owner magnusp, #194: "Public API change; the owner has not decided."

Code and pins. LibsqlResultSetMetaData has a public LibsqlResultSetMetaData(List<Col>) constructor and two package-private constructors. LibsqlResultSet has a public LibsqlResultSet(Statement, StmtResult) constructor and a package-private 4-arg constructor. No commit in any ref contains LibsqlResultSetMetaData(Statement, StmtResult). LibsqlStatement.java:392 and :432 use the public 2-arg LibsqlResultSet constructor for generated keys. The public LibsqlResultSetMetaData(List<Col>) constructor is unused in main and in tests. The public LibsqlResultSet(Statement, StmtResult) constructor is used in main and in tests.

Smallest probe. git log --all -S "LibsqlResultSetMetaData(Statement" returns nothing. grep of the public constructors shows the two shapes above. The named constructor never existed, so the row cannot be executed as written.

Exact question. Which public constructor did you mean, and is the public constructor surface of LibsqlResultSet and LibsqlResultSetMetaData part of the drop-in compatibility promise?

Options.

1. Keep both public constructors. Record that the named (Statement, StmtResult) constructor does not exist, so the row closes as written.
2. Treat LibsqlResultSet(Statement, StmtResult) as the target. Keep it, because LibsqlStatement uses it and tests rely on it. Removing it would need a production change and a test migration.
3. Treat LibsqlResultSetMetaData(List<Col>) as the target. It is unused in main and in tests. Remove it only if the public constructor surface is not part of the promise. No test migration is needed.
4. Declare the constructor surface non-contractual and remove unused public constructors in a breaking release, with a migration note.

No issue is opened until the owner answers. Confidence is high on the premise and blocked on the decision.

Record and limits

The record is #194's "Rejected on purpose" list, the pass PRs #216-#236, and the current tree. The decision trail is .audit/282-rejected-refactors.tsv (local, uncommitted). No code, test assertion, public API, exception type, or message changed. #282 stays open.

The swarm skill was not used. The six rows share one small evidence base and the probes are sequential comparisons. Fan-out would duplicate reads without adding coverage. The rows are independent in principle, but the investigation is small enough that a single trail is clearer.

Open issues #278 and #280 extract parameter streams and generated keys from the statement classes. Neither covers the unsupported-operation blocks or the constraint parsers. #276 covers result-set value reads and explicitly leaves the two NULL methods and the getObject if-chains in place.

One new issue: #283.