[#902] Name the table to the index guard of the JDBC backend the way the database stores it - #1001
Open
vharseko wants to merge 1 commit into
Conversation
…DBC backend the way the database stores it The catalog reads that guard the create table and the create index of openTree() are narrowed to the database and the schema path of their connection since OpenIdentityPlatform#893, which is what OpenIdentityPlatform#902 asked for. Two things were left over. isExistsIndex() named the table to getIndexInfo() as it was written, and the fold an unquoted identifier goes through was applied by its oracle caller alone. getIndexInfo() matches its argument against the stored form, so the rule belongs where isExistsTable() already keeps it - storedIdentifier(), which asks the driver rather than the engine name - and the oracle branch no longer carries an upper case of its own. And the direction OpenIdentityPlatform#902 is about had no case: PgSqlTestCase covered a table of another schema of the search path being found, never a table of a schema off it being passed over. The new case gives a neighbouring directory a table and an index of the same names in a schema this storage does not resolve in, and asserts the open creates both of its own and writes to its own table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #902
Problem
#902 reported both catalog reads of the JDBC backend — the one guarding
create tableand the one guardingcreate index— asking withnullwhere the JDBC contract takes a schema, so an object of another schema of the same database answered for the one this backend was about to create. A table is named after the hash of its tree name and its index after the table, so both names follow from the configuration and from nothing else: two directories sharing one database, each with a schema and asearch_pathof its own, hold a table of the same name and an index of the same name, and nothing about either name tells them apart.Most of that is already fixed on master. #893 gave every lookup of this backend a
TableScope— the database of the connection and the schemas an unqualified name of it resolves in — andisExistsTable(),isExistsIndex()and the leftover listing all read through it. That PR never named #902, so the issue stayed open. What it left behind is what this changes.The table named to
getIndexInfo()is not the name the database storesisExistsIndex()passed the table name as it was written, and the fold an unquoted identifier goes through was applied by one caller alone — the oracle branch ofopenTree(), withtableName.toUpperCase(Locale.ROOT)at the call site:getIndexInfo()takes a name and matches it against the stored form, so the rule belongs in the lookup — whereisExistsTable()already keeps it, instoredIdentifier(), which asks the driver which way it folds rather than matching the engine's class name. On the three engines that reach this guard today the two spellings agree; the knowledge sitting in one branch of the caller is what a fourth engine would inherit nothing of — a lookup that finds no index of a table that carries one, and thecreate indexbehind it reissued. On mysql and oracle, whosecreate indexhas noif not exists, that is the open of the tree failing.The direction #902 is about had no case
testAClearFindsATableOfAnotherSchemaOfTheSearchPathcovers the other half of the narrowing: a table of a schema on the path must still be found, and the open must not create a second one shadowing it. Nothing covered a table or an index of a schema off the path being passed over — the collision the issue was filed about.Fix
isExistsIndex()names the table the way the database stores it, through the samestoredIdentifier()the table lookup takes, and the metadata is read once for both.openTree()no longer folds for itself. Its comment said "unquoted identifiers are stored in uppercase", which is true of that engine and not of the guard's business: the driver is asked instead, exactly asisExistsTable()asks it.Nothing about the scope of either lookup changes — that is #893's and stays as it is.
Tests
PgSqlTestCase.testAnOpenIsAnsweredForByNoTableOfASchemaOffTheSearchPath— the case of #902. A neighbouring directory is spelled out by hand in a schema this storage's connections do not resolve in: the same tableopendj_<hash>and the same indexk_<hash>. The open must create both of its own in the schema it works in, and the write of the transaction must land in its own table (the neighbour's row count is asserted to stay zero). The fixture is made by hand rather than by a second storage, because what the case needs is a schema this one does not reach — and a storage of this suite creates its tables in the schema it does.JDBCStorageRetryTest.testTheIndexGuardNamesTheTableAsTheDatabaseStoresIt— over a mocked engine whose metadata reports that it stores identifiers folded upwards, the guard has to ask about the folded name, whichever branch ofopenTree()the driver took to get there.Watched failing
Both were run against the guard put back the way it was, one half at a time:
the open took the index of a schema it does not reach unqualified for its own: the cursor batches of this tree are full scans behind it— the quiet half of the issue, the table half passing beside itPSQLException: ERROR: relation "opendj_4f91…" does not exist— the loud half, raised by the first statement against a table that was never createdgetIndexInfoasked"opendj_1809…"where the driver says the database stores"OPENDJ_1809…"What was run
JDBCStorageRetryTestPgSqlTestCaseMySqlTestCaseOracleTestCasegvenzl/oracle-freeup within its 5 minute startup wait (two attempts), so all 80 cases skippedSince the oracle branch is the one whose hard-coded fold this PR removes, the assumption it now rests on was probed directly against a hand-started
gvenzl/oracle-free:The fold is required there, and
storedIdentifier()produces exactly the name the call site used to spell out.