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.

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

It's not possible to modify some tables #1733

Closed
2 of 4 tasks
gozzoo opened this issue Feb 8, 2019 · 12 comments
Closed
2 of 4 tasks

It's not possible to modify some tables #1733

gozzoo opened this issue Feb 8, 2019 · 12 comments
Assignees
Labels
bug Confirmed bugs or reports that are very likely to be bugs.
Milestone

Comments

@gozzoo
Copy link

gozzoo commented Feb 8, 2019

Details for the issue

What did you do?

I opened a database with few existing tables. All tables did have multiple fields

What did you expect to see?

image

This is with version 3.10.1

What did you see instead?

For some tables there is no expand icon.

image

When I right-click the name of such table and select 'Modify table' I see defintion for an empty table.

image

Useful extra information

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

What operating system are you using?

  • Windows: (Windows 7 32 bit)

What is your DB4S version?

  • 3.11.0

Did you also

@chrisjlocke
Copy link
Member

The schema for those tables might contain some weird characters? Is it possible to email me the database? db4s [at] chrisjlocke [dot] co [dot] uk. You can send me a database sans data (executing the necessary 'delete from ...' commands from the 'execute sql' tab should still work, regardless of whether its fully read the schema)

@gozzoo
Copy link
Author

gozzoo commented Feb 8, 2019

I updated my report. It works fine with version 3.10.1

Here is the table deifnition:

CREATE TABLE "auto_measurement" (
	`id`	INTEGER NOT NULL,
	`full`	BOOLEAN,
	`type`	INTEGER,
	`entry`	INTEGER DEFAULT (DATETIME(CURRENT_TIMESTAMP,'LOCALTIME')),
	`exit`	INTEGER,
	`plate`	char ( 50 ),
	`seller`	CHAR ( 50 ),
	`buyer`	CHAR ( 50 ),
	`driver`	CHAR ( 50 ),
	`storage`	TEXT,
	`storage2`	TEXT,
	`trailer`	CHAR ( 50 ),
	`product`	TEXT,
	`product_code`	TEXT,
	`tara`	NUMERIC,
	`bruto`	NUMERIC,
	`neto`	NUMERIC,
	`discount`	NUMERIC DEFAULT 0,
	`price_kg`	NUMERIC,
	`note`	TEXT,
	`carrier`	char ( 50 ),
	`subcontractor`	char ( 50 ),
	`site`	INTEGER,
	`user1`	TEXT,
	`user2`	TEXT,
	`user3`	TEXT,
	`operator_in`	TEXT,
	`operator_out`	TEXT,
	`status`	INTEGER,
	`scale`	TEXT,
	`manual`	INTEGER,
	`price`	NUMERIC,
	PRIMARY KEY(`id`)
);

@chrisjlocke
Copy link
Member

chrisjlocke commented Feb 8, 2019

I assume you're aware thats not a SQLite compatible table definition. SQLite doesn't understand what 'char' is, or 'boolean'. It guesses, but it certainly isn't going to respect the 50 character field limits. I'm guessing its the 'entry' line causing grief - if you remove that, it works fine. It does appear valid ... its just our parser can't parse it. Doh.

(See also: https://stackoverflow.com/questions/200309/sqlite-database-default-time-value-now)

@chrisjlocke chrisjlocke added the bug Confirmed bugs or reports that are very likely to be bugs. label Feb 8, 2019
@justinclift
Copy link
Member

@gozzoo Out of curiosity, which database dialect is that SQL table definition created for?

@gozzoo
Copy link
Author

gozzoo commented Feb 9, 2019

@justinclift No specific dialect in particular. I was expecting our projectt needs to outgrow sqlite pretty soon and wanted the database scripts to be portable. I never experimented running them on another dabtabase.

MKleusberg added a commit that referenced this issue Feb 9, 2019
Remove the window function grammar rules. I think they are not needed
anyway as long as we only parse CREATE TABLE and CREATE INDEX
statements and unexpectedly they do seem to cause problems. It it still
worth investigating how this is possible but for now removing them seems
like the best option.

See issue #1733.
MKleusberg added a commit that referenced this issue Feb 9, 2019
Add a test case for the bug which motivated disabling the window
function rules in ee70a34.

See issue #1733.
@MKleusberg MKleusberg self-assigned this Feb 9, 2019
@MKleusberg
Copy link
Member

This is a very weird bug. Looks like it was introduced by adding some more rules to our grammar parser. And even when these rules aren't used anywhere they cause some problems. I have disabled them now which shouldn't cause any problems since we don't use them (yet). So with the next nightly build everything should work as expected again.

Can you download the nightly build tomorrow and double check, @gozzoo? 😄 If it's working again in the nightly this is definitely a regression from 3.10.1 and the fix should definitely be included in a 3.11.1 release 😉

@justinclift justinclift added this to the 3.11.1 milestone Feb 9, 2019
@justinclift
Copy link
Member

That reminds me... the build server has been off all day, so the nightlies haven't run yet. I'll manually kick off the build scripts now.

Also just created a 3.11.1 milestone, so we have something to add this to. Made the due date two weeks from now, but we can adjust that sooner/later if need be. 😄

@justinclift
Copy link
Member

@gozzoo The Win64 nightly builds just finished, so they're online and ready for testing if/when you have a chance:

    https://nightlies.sqlitebrowser.org/latest/

@gozzoo
Copy link
Author

gozzoo commented Feb 11, 2019

It's working fine now, thanks.

@chrisjlocke
Copy link
Member

Thanks for letting us know. Will close this as its resolved (of sorts).

MKleusberg added a commit that referenced this issue Feb 13, 2019
Remove the window function grammar rules. I think they are not needed
anyway as long as we only parse CREATE TABLE and CREATE INDEX
statements and unexpectedly they do seem to cause problems. It it still
worth investigating how this is possible but for now removing them seems
like the best option.

See issue #1733.
@MKleusberg
Copy link
Member

I've cherry-picked this over to the 3.11.x branch so it will be included in the 3.11.1 release 😄 Thanks again for pointing this out, @gozzoo!

@gozzoo
Copy link
Author

gozzoo commented Feb 14, 2019

Thanks @MKleusberg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs.
Projects
None yet
Development

No branches or pull requests

4 participants