/sqlitebrowser

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.

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

problem parsing check constraint table definition #1454

Open
eyeteajay opened this Issue Jun 29, 2018 · 5 comments

Comments

Assignees

@MKleusberg MKleusberg

Labels
SQL bug
Projects
None yet
Milestone
No milestone
6 participants
@eyeteajay @MKleusberg @parno @mgrojo @moll @justinclift
@eyeteajay
Copy link

eyeteajay commented Jun 29, 2018 •
edited

Under the table definition, there is a warning that says: "There is something wrong with this table definition that our parser doesn't fully understand." Also, the table cannot be expanded in the "database structure panel". However, the program seems to work and read the rows anyway.

Details for the issue

The problem is with ck_AssetTreeAssoc_depth, because the table definition can be parsed fine without it. My guess would be the nested parenthesis? The problem DDL is:

CREATE TABLE "AssetTreeAssoc" (
	uuid BINARY(16) NOT NULL, 
	cdt DATETIME DEFAULT (DATETIME('NOW')) NOT NULL, 
	mdt DATETIME DEFAULT (DATETIME('NOW')) NOT NULL, 
	"passetID" INTEGER NOT NULL, 
	"cassetID" INTEGER NOT NULL, 
	depth INTEGER DEFAULT 0 NOT NULL, 
	id INTEGER NOT NULL, 
	PRIMARY KEY (id), 
	CONSTRAINT "ck_AssetTreeAssoc_depth" CHECK ((passetID <> cassetID and depth > 1) or depth == 0),
	CONSTRAINT "uq_AssetTreeAssoc" UNIQUE ("cassetID", depth), 
	FOREIGN KEY("passetID") REFERENCES "AssetTree" (id) ON DELETE CASCADE, 
	FOREIGN KEY("cassetID") REFERENCES "AssetTree" (id) ON DELETE CASCADE
)

Useful extra information

The info below often helps, please fill it out if you're able to. :)

What operating system are you using?

  • Windows: ( version: ___ )
  • Linux: ( distro: ___ )
  • Mac OS: ( _version: 10.11.6 )
  • Other: ___

What is your DB4S version?

  • 3.10.1
  • 3.10.0
  • 3.9.1
  • Other: ___

Did you also

@justinclift justinclift added the help wanted label Jun 29, 2018

@MKleusberg

This comment has been minimized.

Sign in to view
Copy link
Member

MKleusberg commented Jul 10, 2018

Thanks for the report and the detailed analysis, @eyeteajay! This is a problem in our grammar parser. We detect these sort of issues (which is why you get that warning) and fall back to a simpler approach. That's why you can still browse the table. But for other features and especially for editing the table there just isn't enough information this way.

Anyway, I'll have a closer look at the problem now 😄

@MKleusberg MKleusberg added bug and removed help wanted labels Jul 10, 2018

@MKleusberg MKleusberg self-assigned this Jul 10, 2018

MKleusberg added a commit that referenced this issue Jul 10, 2018

@MKleusberg
grammar: Fix parsing of complex expression 
This fixes parsing of expressions of the form '(x) AND/OR y' and similar
types. It also fixes expression of the '(x)' type and of the '(x op y
AND z op w)' type.

See issue #1454.
Loading status checks…
788134e
@MKleusberg

This comment has been minimized.

Sign in to view
Copy link
Member

MKleusberg commented Jul 10, 2018

I believe the issue is fixed now 😄 Can you try tomorrow's nightly build and check if it's working for you, @eyeteajay? It's also worth checking if you have issues with any other tables.

@MKleusberg MKleusberg added the response requested label Jul 12, 2018

@parno

This comment has been minimized.

Sign in to view
Copy link

parno commented Jul 30, 2018 •
edited

FWIW, I'm seeing a similar issue with this (simplified) table definition:

CREATE TABLE "Transactions" (
	"stock_id"	INTEGER CHECK(
      (stock_id is null AND shares is null)
      OR
      (stock_id is not null AND shares is not null)),
	"shares"	INTEGER
);

I was able to create it via the UI, but the parser seems to struggle with it, even with today's (7/29) nightly build. I'm running on Mac OS X, 10.13.5.
@mgrojo

This comment has been minimized.

Sign in to view
Copy link
Contributor

mgrojo commented Oct 8, 2018

I tested with the queries in this issue. The first one couldn't be parsed until I removed the DEFAULT clauses:

CREATE TABLE "AssetTreeAssoc4" (
	uuid BINARY(16) NOT NULL, 
	cdt DATETIME, -- DEFAULT (DATETIME('NOW')) NOT NULL, 
	mdt DATETIME, -- DEFAULT (DATETIME('NOW')) NOT NULL, 
	"passetID" INTEGER NOT NULL, 
	"cassetID" INTEGER NOT NULL, 
	depth INTEGER DEFAULT 0 NOT NULL, 
	id INTEGER NOT NULL, 
	PRIMARY KEY (id), 
	CONSTRAINT "ck_AssetTreeAssoc_depth" CHECK ((passetID <> cassetID and depth > 1) or depth == 0),
	CONSTRAINT "uq_AssetTreeAssoc" UNIQUE ("cassetID", depth), 
	FOREIGN KEY("passetID") REFERENCES "AssetTree" (id) ON DELETE CASCADE, 
	FOREIGN KEY("cassetID") REFERENCES "AssetTree" (id) ON DELETE CASCADE
)

The second one has problems too. It worked when one side of the OR condition is entirely removed:

CREATE TABLE "Transactions3" (
	"stock_id"	INTEGER CHECK(
      stock_id is null AND shares is null),
--      OR
--      (stock_id is not null AND shares is not null)),
	"shares"	INTEGER
);

Removing the internal parenthesis is also parseable (of course, I'm not proposing this as a workaround):

CREATE TABLE "Transactions4" (
	"stock_id"	INTEGER CHECK(
      stock_id is null AND shares is null
      OR
      stock_id is not null AND shares is not null),
	"shares"	INTEGER
);

@mgrojo mgrojo added SQL and removed response requested labels Oct 8, 2018

@moll

This comment has been minimized.

Sign in to view
Copy link

moll commented Dec 1, 2018

Also seeing the same here with v3.11.0-beta1 and the following schema:

CREATE TABLE initiatives (
  uuid STRING PRIMARY KEY NOT NULL,
  mailchimp_interest_id STRING NOT NULL UNIQUE,
  notes TEXT NOT NULL DEFAULT "",

  CONSTRAINT initiatives_uuid_length
  CHECK (length(uuid) == 36),

  CONSTRAINT initiatives_mailchimp_interest_id
  CHECK (length(mailchimp_interest_id) > 0)
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment