fix(datagrid): let a SQLite column declared with no type carry a label and a search - #3022
Merged
Merged
Conversation
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
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.
Found while investigating #2996.
#3009 changed the same file and has since merged, so this is rebased onto it and stands alone.
The defect
create table t(a, b)is legal SQLite and common in hand-written databases. Measured on SQLite 3.54.0:The declared type comes back as a zero-length string, not null, so the column is kept and
dataTypereaches the app as"".ColumnTypeClassifierfalls through every lookup to its.textfallback, giving.text(rawType: ""), and then:baseTypeNamereturns nil for an empty name, sosupportsPatternMatchis false. That has two consequences in the foreign key picker, and neither is reported:ForeignKeyLabelColumn.resolvefilters the column out of its candidates, so a table whose columns are all untyped offers no label at all and the picker lists bare keys.ForeignKeyLookupQuery.searchFiltersgates the predicate on the same property, so the search silently matches nothing.Repro, measured:
CREATE TABLE marchi("marchio", "nome"); CREATE TABLE alimenti(id INTEGER PRIMARY KEY, marchio, FOREIGN KEY(marchio) REFERENCES marchi(marchio));Open the picker onalimenti.marchioand it listsBRD,MULwith nothing beside them, thoughnomeholdsBarillaandMulino Bianco.Why the closed list was right and still is
The list is deliberately closed.
ColumnTypeClassifierfilesuuid,inetand everything else it does not recognise under.text, andLIKEagainst auuid, an enum or an array is an error on PostgreSQL rather than an empty result, so a name that is not a known character type carries no predicate.An empty declared type is not a gap in that list. It is a different answer, and only a dynamically typed engine gives it: every strict engine always names a type, and SQLite is the one that does not.
LIKEis defined on every column there, measured on the same build:So the fix is one branch, not a new entry in the list: a column whose declared type is present and empty takes a predicate.
What it deliberately does not do
A missing
rawTypestays untouched..text(rawType: nil)is built by several of the app's own conversions (JsonRowConverter,InClauseConverter,MultiRowEditState), and that is the app having no type information, not an engine reporting a column declared without one. Reading the two the same way would hand a pattern predicate to a column nobody has typed, on any engine.declaresNoTypeis the distinction and it is tested in both directions.Verified
verify.sh generateverify.sh test(4 suites)verify.sh lint(2 files)ForeignKeyLookupColumnTestsis new and pins the whole predicate, not just the new branch: the character types still match, the guessed ones still do not, a non-text type still does not, an empty declared type now does, a missing one still does not, and the reporter's all-untyped table now resolvesnomeas its label.