sql: name resolution woes post-2.0 #24598
Assignees
Labels
A-sql-pgcompat
Semantic compatibility with PostgreSQL
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
C-cleanup
Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.
Milestone
Comments
|
cc @vivekmenezes I'll be working on this soon/next (after closing my current UPSERT PR), as this corresponds to one "unit of work" conceptually and contains fixes to known 2.0 regressions |
|
cc @jordanlewis |
knz
added
C-bug
|
All 2.0-critical issues now have PRs. The relevant PRs will be cherry-picked to 2.0.x. The remaining items can be addressed for 2.1. |
|
The two remaining issues are not directly related to name resolution. We'll catch them in separate sub-projects. Closing this now. |
craig bot
pushed a commit
that referenced
this issue
May 22, 2018
24735: sql: sanitize the "no current db" story r=knz a=knz Fixes #23893. Fixes #23145. Updates #24598. Informs #24056. Informs #23958. Prior to this patch, CockroachdB made it excessively simple for client connections to not have any current database set. Also by design CockroachDB allows a connection to be opened with a non-existent database set as current. This in turn causes various problems related to name resolution and the `pg_catalog` schema. To understand these problems we can consider separately two groups of users, which have separate use cases: - newcomers to CockroachDB that don't have prior experience with SQL or PostgreSQL. These will not know about "databases" in any sense, and will use CockroachDB with default options. This includes using `cockroach sql` with no current database selected. These users have to be taught upfront that they need to "create a database" and "set the current database", which they may or may not (more likely not) be interested in. When these users then transition into writing apps, they will copy-paste the conn URL printed by `cockroach start` (which sets no current db) into some ORM or framework, and then will regretfully observe that their app fail in mysterious ways because the current db is not set (also see the next point). - users of existing applications or SQL drivers that have been developed to target PostgreSQL. These apps commonly contain code that: - connect to a database named "`postgres`" by default, and make this default hard(er) to customize. For example Ecto (Elixir ORM) doesn't make this configurable. (From the PostgreSQL docs: "After initialization, a database cluster will contain a database named `postgres`, which is meant as a default database for use by utilities, users and third party applications. The database server itself does not require the postgres database to exist, but many external utility programs assume it exists.") - (regardless of which db is selected as current), will issue some introspection queries using `pg_catalog` before any additional initialization is made by the higher-level app code, in particular before the app can issue `CREATE DATABASE`. Currently `pg_catalog` queries (and several other things) fail if the current database is unset or set to a non-existing database. To address this relatively large class of problems, this patch modifies CockroachDB as follows: - all clusters (existing or new) now get two empty databases on startup (created by a migration) called `defaultdb` and `postgres`. - when a client doesn't specify a current db in their conn string (i.e. in the pgwire options), the `defaultdb` database is picked up instead. This resolves all the known problems around the situations identified above. The two new databases behave like any regular database and are provided for convenience to newcomers and compatibility. (Administrators are free to drop them afterwards to restore the previous situation, if that is actively desired.) In addition, to make it slightly harder for newcomers to shoot themselves in the foot, the `database` session variable cannot be set to the empty string any more if `sql_safe_updates` is otherwise set. Three alternatives to this approach were considered, discussed with @jordanlewis and @nvanbenschoten in their quality of pg compatibility experts, and with @bdarnell in his quality of carer for newcomers, and ultimately rejected: A) generate the current db upon connection if it does not exist, make connections using the empty string as curdb use `system` instead. Rejected because we don't like to auto-create upon connection. Too easy to flood the system with bogus database names by typos in the connection string. B) Pre-populate clusters with a special anonymous database (db name = empty string). Rejected because there are too many areas in CockroachDB with a risk to fail silently or with weird error messages as a result. Also does not solve the problem of clients that want a db called `postgres`. C) Auto-create a database named after the user upon user creation (with an optional flag to skip db creation). Rejected because it does not solve the problem of clients that want a db called `postgres`. @bdarnell: creating a database per user also implicitly encourages bad habits (like sharing certs for the user whose name corresponds to the database instead of creating multiple users). D) Implement some magic in the handling of `pg_catalog`, name resolution and all other pieces that currently don't like non-existent `database` values to create the appearance of something that works. Rejected because too complex to implement successfully (there are many moving parts that need to be touched to make this work) and the resulting magic would be hard to understand by CockroachDB contributors and hard to maintain over time. E) Auto-create only one database called `defaultdb`. Rejected because it does not solve the problem of clients that want a db called `postgres`. F) Auto-create only one database called `postgres` and use that as default when no current db is specified. Rejected because not good for branding. G) Name the `defaultdb` either `def`, `default` or `default_database`. The word "`default`" was rejected because it is a SQL keyword and would cause queries using it to fail in obscure ways. The word "`def`" (similar to what MySQL uses in `information_schema`) was rejected because it refers too strongly to other uses of this word in programming languages ("define"). "`default_database`" was rejected because too verbose. Release note (sql change): new and existing clusters will now contain two empty databases called `defaultdb` and `postgres` by default. The database `defaultdb` is automatically used for clients that connect without a current database set (e.g. without a database component in the connection URL). The database `postgres` is provided for compatibility with PostgreSQL client frameworks that require it to exist when the database server has ben freshly installed. Both new databases behave like any other regular database. Release note (general change): existing clusters upgraded to this version of CockroachDB will automatically see two new empty databases named `defaultdb` and `postgres`. These were added for compatibility with PostgreSQL and to ease adoption of CockroachDB. They can be manually deleted by an administrator if the client applications are determined not to require them. Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is a meta-issue that describes remaining name resolution problems discovered around the 2.0 release.
Issues due to invalid/missing default db
These issues can be solved by either introducing a pseudo-descriptor for the current database when no valid current db is set, or ensuring the current db exists when a session is opened.
pg_catalogerror out cleanly when no cur db set sql: makepg_catalogerror out cleanly when no cur db set #23958Outright pg compat bugs
Unfortunate bugs due to the special CRDB 1.x compatibility "feature"
Other bugs due to CockroachDB idiosyncrasies
UX improvements
The text was updated successfully, but these errors were encountered: