Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql/coltypes: remove the field Name in TArray #28959

Merged
merged 4 commits into from
Aug 23, 2018
Merged

sql/coltypes: remove the field Name in TArray #28959

merged 4 commits into from
Aug 23, 2018

Commits on Aug 23, 2018

  1. sql: fix the handling of string types 

    This patch corrects two long-standing bugs in CockroachDB which were
confusing both 3rd party tools/ORMs and users:

- the SQL standard type called "CHAR" is supposed to have a default
  maximum width of 1 character.

  Prior to this patch, CockroachDB did not support this and instead
  set no default maximum width on CHAR. This would cause CockroachDB
  to fail validating the width of values inserted in tables, fail
  to constrain the width of values during casts to CHAR, and report
  incorrect information in introspection tables.

- PostgresSQL's dialect distinguishes the table column types TEXT,
  CHAR, VARCHAR and a special thing called `"char"` (this is a legacy
  PostgreSQL type which is, unfortunately, still used by some
  pg_catalog tables).

  Prior to this patch, CockroachDB would map all of these types into
  the same column type "STRING". In turn this caused them to show
  up as "text" in introspection.

  While this was not incorrect from the perspective of value
  handling (all these types are, indeed, handled with about the same
  value semantics in both PostgreSQL and CockroachDB), ORMs
  do pay attention to this distinction and become confused if they
  see a different type in introspection (e.g. "text") than what
  the application model requires (e.g. "char"). The result of this
  confusion was to trigger a schema change to adapt the type, which
  in turn fails due to missing features in ALTER COLUMN TYPE.

This patch corrects both problems by giving the appropriate default
width to CHAR and preserving the distinction between the various string
types in column descriptors and thus introspection.

Additionally, a width constraint check on collated string columns was
missing. This is now fixed too.

Tables and columns created prior to this patch are unaffected.

Release note (bug fix): CockroachDB now distinguishes "CHAR" and
"VARCHAR" as mandated by the SQL standard and PostgreSQL
compatibility. When a width is not specified (e.g. `CHAR(3)`), the
maximum width of `VARCHAR` remains unconstrained whereas the maximum
width for `CHAR` is now 1 character.

Release note (bug fix): CockroachDB now properly checks the width of
strings inserted in a collated string column with a specified width.

Release note (sql change): CockroachDB now preserves the distinction
between different column types for string values like in PostgreSQL,
for compatibility with 3rd party tools and ORMs.
    @knz
    knz committed Aug 23, 2018
    Configuration menu
    Copy the full SHA
    9c63fe4 View commit details
    Browse the repository at this point in the history

  2. sql: desugar JSON to JSONB 

    The distinction between the type names "JSON" and "JSONB" is
inconsequential. The types are already reported properly in
introspection using the right alias. This patch removes this
distinction in code.

Release note: None
    @knz
    knz committed Aug 23, 2018
    Configuration menu
    Copy the full SHA
    569c9bd View commit details
    Browse the repository at this point in the history

  3. sql: desugar "TIMESTAMP WITH TIME ZONE" to "TIMESTAMPTZ" 

    Prior to this patch the canonical name for the timestamptz type in
CockroachDB was "TIMESTAMP WITH TIME ZONE" with spaces and all
glorious 24 characters.

This is unfortunate because this is a pretty common type, and is
moreover used to disambiguate the return type of `now()` and thus will
be emitted pretty often in distsql physical plans. Having such a long
type name is unnecessary and indeed adds 50%+ storage, communication
and parsing overhead for every occurrence of this type in
distsql plans.

This patch shortens the canonical CockroachDB name to "TIMESTAMPTZ".

The representation of the type in introspection tables (pg_catalog,
information_schema) is unaffected.

Release note: None
    @knz
    knz committed Aug 23, 2018
    Configuration menu
    Copy the full SHA
    a18647e View commit details
    Browse the repository at this point in the history

  4. sql/coltypes: remove the field Name in TArray 

    It was not needed. This simplifies.

Release note: None
    @knz
    knz committed Aug 23, 2018
    Configuration menu
    Copy the full SHA
    f64a081 View commit details
    Browse the repository at this point in the history