Skip to content

Accept only SQL-standard type names in as_sqlr_type() - #12

Draft
nbenn wants to merge 3 commits into
mainfrom
9-standard-types
Draft

nbenn wants to merge 3 commits into
mainfrom
9-standard-types

Conversation

@nbenn

@nbenn nbenn commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • Type strings must now be one of the SQL-standard spellings from the issue's table, in any case and spacing. Anything else, engine aliases and a bare float included, is an error pointing to ?as_sqlr_type and sqlr_other(), instead of a silent sqlr_other().
  • The new parser also fixes three misreadings: the old one dropped with time zone, read binary(n) as variable-length and left char unsized. A parsed type now equals what its constructor builds, with raw left for reflection to set, as ?sqlr_type documents.
  • Types gain a format() method that runs the same table in reverse and falls back to the constructor call, such as sqlr_integer_type(bytes = 1L). A type difference in sqlr_diff() now reads varchar(255) vs text, and equality compares constructor calls, replacing type_key().
  • The package now calls S7::methods_register() in .onLoad(). Without it an installed sqlr never registers S7 methods on base generics, so format() would only have worked under load_all(), and the existing print() method for sqlr_sql has been ignored all along. The format tests guard this under R CMD check, which runs them against the installed package.
  • No dialect implements sqlr_parse_type(), so it is dropped, along with the utils import only the old parser used. Neither dialect has called as_sqlr_type() since Reflect column types with this dialect's own mapping sqlr.sqlite#2, so the stricter parsing reaches authored strings only.

Spellings that parsed before and are now refused:

Refused Write instead
int2, int4, int8 smallint, integer, bigint
float4, float8, double, bare float real, double precision or float(p)
bpchar char(n)
bytea blob
bool boolean
timetz, timestamptz time with time zone, timestamp with time zone
jsonb sqlr_json(binary = TRUE)
varchar without a length varchar(n), or text if unbounded
Unmodelled types, such as geometry sqlr_other("geometry")

Fixes #9

S7 registers methods for generics from other packages, base's print()
and format() included, only when methods_register() runs at load time.
Without it the installed package ignored its print() method for
sqlr_sql.
Neither dialect has a method for it: each reads types inside its
sqlr_reflect_schema() method, which is where the reverse mapping lives.
The lookup table mixed spellings from several engines and would have
grown with every dialect, while an unknown name quietly became
sqlr_other(), so a typo or a bare float could never compare equal to
its reflection. Anything outside the standard set is now an error, and
format() runs the table in reverse, which sqlr_diff() now prints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accept only SQL-standard type names in as_sqlr_type()

1 participant