mayerwin / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
0 stars 0 forks source link

Cannot Re-edit Table (remove/rename columns, change column type) #12

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. Create table "names".
2. Create two columns in the table -- first name and last name.
3. Try to edit the table and remove one of the columns, even with no data in it.
4. Says successful, but doesn't do it.

What is the expected output? What do you see instead?

Successful column deletion. I saw unsuccessful column deletion.

What version of the product are you using? On what operating system?

1.8.2 on Ubuntu 10.04 Linux

Please provide any additional information below.

Love what I see so far. Amazing that it's all in one single file.

Original issue reported on code.google.com by volom...@gmail.com on 20 Apr 2011 at 5:19

GoogleCodeExporter commented 9 years ago
Could you tell me what version of PHP, what version of SQLite, what library 
extension you are using (PDO, SQLite3, SQLiteDatabase, etc.), and and version 
that library extension is at? Following the steps you described does not cause 
a problem - it works fine and the column gets deleted as it should. This means 
there is a difference between your setup and my setup.

I'm glad you like the tool! The single-fileness is definitely a plus.

Original comment by daneirac...@gmail.com on 20 Apr 2011 at 5:42

GoogleCodeExporter commented 9 years ago
PHP 5.3.2-1ubuntu4.7

PDO

PDO support => enabled
PDO drivers => mysql, pgsql, sqlite, sqlite2

pdo_sqlite

PDO Driver for SQLite 3.x => enabled
SQLite Library => 3.6.22

SQLite

SQLite support => enabled
PECL Module version => 2.0-dev $Id: sqlite.c 293036 2010-01-03 09:23:27Z 
sebastian $
SQLite Library => 2.8.17
SQLite Encoding => UTF-8

Directive => Local Value => Master Value
sqlite.assoc_case => 0 => 0

sqlite3

SQLite3 support => enabled
SQLite3 module version => 0.7-dev
SQLite Library => 3.6.22

Directive => Local Value => Master Value
sqlite3.extension_dir => no value => no value

Original comment by volom...@gmail.com on 20 Apr 2011 at 5:46

GoogleCodeExporter commented 9 years ago
So, on the main page for the database, what are the values for:

SQLite version: x
SQLite extension: y

?

They should be x=3 and y=PDO

Is that right?

Original comment by daneirac...@gmail.com on 20 Apr 2011 at 6:01

GoogleCodeExporter commented 9 years ago
SQLite version: 3
SQLite extension: PDO

Yes, correct.

Original comment by volom...@gmail.com on 20 Apr 2011 at 6:04

GoogleCodeExporter commented 9 years ago
I have no idea why this is happening for you. This seems to be a common theme 
with these issues - reproducible by one person, and not for another. Ian, do 
you know what's up...

Original comment by daneirac...@gmail.com on 20 Apr 2011 at 7:31

GoogleCodeExporter commented 9 years ago
Same issue here.

SQLite version: 3
SQLite extension: PDO
PHP version: 5.3.1

Error I get when I try to delete is: An error occured. This may be a bug that 
needs to be reported at code.google.com/p/phpliteadmin/issues/list

Original comment by gyss...@gmail.com on 27 Apr 2011 at 12:24

GoogleCodeExporter commented 9 years ago
Just tested on a cPanel install as well, didn't work:

SQLite version: 3
SQLite extension: PDO
PHP version: 5.3.4

Original comment by gyss...@gmail.com on 27 Apr 2011 at 1:16

GoogleCodeExporter commented 9 years ago
SQLite currently does not allow a full set of ALTER TABLE options. The way that 
phpLiteAdmin allows editing, deleting, and adding columns after the table has 
already been created is complicated and involves creating temporary tables and 
other hacks. 

This area of the tool is the most delicate, and I'm not surprised that there 
are problems with it. Could you describe the structure of the table before 
trying to delete, which column you are deleting, and any other relevant info?

Original comment by daneirac...@gmail.com on 28 Apr 2011 at 4:31

GoogleCodeExporter commented 9 years ago
Is there a way i can e-mail you? I'll send you the URL to a running version of 
phpliteadmin on my box, and you can try to create/delete a simple two column 
table. You should get the error, there's nothing complicated about the tables.

Original comment by gyss...@gmail.com on 28 Apr 2011 at 5:33

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Sure go ahead and email me at daneiracleous@gmail.com. 

Original comment by daneirac...@gmail.com on 31 May 2011 at 8:00

GoogleCodeExporter commented 9 years ago
Did you email me yet?

Original comment by daneirac...@gmail.com on 8 Jun 2011 at 3:24

GoogleCodeExporter commented 9 years ago
I just tested again and this bug is unresolved on the latest stable version 
1.8.6 that I just downloaded last week.

OS: Ubuntu 10.04.2 LTS
Kernel: Linux 2.6.32-32-generic #62-Ubuntu SMP Wed Apr 20 21:54:21 UTC 2011 
i686 GNU/Linux
SQlite version: 3
SQLite extension: PDO
PHP version: 

PHP 5.3.2-1ubuntu4.9 with Suhosin-Patch (cli) (built: May  3 2011 00:43:34) 
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
    with the ionCube PHP Loader v4.0.8, Copyright (c) 2002-2011, by ionCube Ltd.

There's nothing special about the table. I explain everything in the original 
report, along with the steps to recreate the problem.

Original comment by volom...@gmail.com on 8 Jun 2011 at 9:05

GoogleCodeExporter commented 9 years ago
Perhaps you can show me how to set the script in debug mode so that all errors 
are written out?

Original comment by volom...@gmail.com on 8 Jun 2011 at 9:20

GoogleCodeExporter commented 9 years ago
I'm also getting this.

Original comment by darktem...@gmail.com on 27 Aug 2011 at 8:52

GoogleCodeExporter commented 9 years ago
There's a debug variable available in v1.8.9 that might help you.

Original comment by daneirac...@gmail.com on 25 Oct 2011 at 10:14

GoogleCodeExporter commented 9 years ago
I have the same problem, but not when deleting a column but when renaming it or 
changing the data type. Sometimes it says the table was altered successfully, 
but nothing happens. Sometimes an error happens.

How I can reproduce at the moment:
1. CREATE TABLE test_table(test_column INTEGER PRIMARY KEY NOT NULL)
2. edit the column test_column into test_column2
3. error: Error: near "test_column2": syntax error.

SQLite version: 3.7.7.1
SQLite extension [?]: PDO
PHP version: 5.3.8

Original comment by crazy4ch...@gmail.com on 3 Dec 2011 at 4:05

GoogleCodeExporter commented 9 years ago
Issue 101 has been merged into this issue.

Original comment by crazy4ch...@gmail.com on 2 Jun 2012 at 8:21

GoogleCodeExporter commented 9 years ago
This bug is just very annoying, so I mark it high-priority now.

Original comment by crazy4ch...@gmail.com on 2 Jun 2012 at 8:40

GoogleCodeExporter commented 9 years ago
Agree with you. Basic functionality ))

03.06.2012 02:40, phpliteadmin@googlecode.com написал:

Original comment by mas...@remort.net on 4 Jun 2012 at 5:57

GoogleCodeExporter commented 9 years ago
Including all associated problems in the title as quested in duplicate issue 
#101.

Also, here is a good link posted by master@remort.net (just so it is not 
lost...):

http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-d
atabase-table

Original comment by crazy4ch...@gmail.com on 4 Jun 2012 at 5:46

GoogleCodeExporter commented 9 years ago
I am currently working on this issue. I could not believe my eyes when I saw 
the way phpliteadmin implements this (or, to be precise, the code we borrow for 
doing this: http://code.jenseng.com/db/). The code is so naive that it hurts.

While debugging this, I am nearly doing a complete rewrite.
Dropping columns already works pretty reliably for SQLite3. SQLiteDatabase 
still causes some typing problems (never thought I'd have typing problems in 
SQLite...).
I am working on changing columns at the moment. Adding columns and rename 
should be pretty simple then.

Original comment by crazy4ch...@gmail.com on 6 Oct 2012 at 11:11

GoogleCodeExporter commented 9 years ago
More progress on this issue. Dropping, changing and adding (for SQLiteDatabase) 
"works" now. It still needs some debugging, but I will soon give you a version 
to test this. It will need a lot of testing, as the whole approach of parsing 
SQL really depends a lot on the syntax used and SQL allows for several 
variations I might not have thought of yet.

After the currently "implemented" features have been debugged, we might add new 
features like:
- add a new column somewhere in the middle (quite simple)
- changing column constraints (NOT NULL, auto increment, default value)
- moving columns
- ...

Original comment by crazy4ch...@gmail.com on 7 Oct 2012 at 11:26

GoogleCodeExporter commented 9 years ago
Okay, so I committed my current version of the reworked ALTER TABLE support to 
svn. You can get it here:
https://phpliteadmin.googlecode.com/svn/source/1.9.3/phpliteadmin.php

I'd be pleased if somebody tested this.
It is not perfect yet. Some remarks:
- backup your db before using this! (But the current stable version is even 
more dangerous I'd say ;-) )
- does not implement table-renames yet (only affects SQLite2, SQLite3 can do 
this on its own)
- will drop indexes/triggers associated with the table (except primary keys)

If it does not work, please provide:
- the CREATE TABLE statement of the table you tried to alter (you can get this 
on the structure-tab)
- the action you tried to perform (drop/change/add? which columns?)
- what it did (error? no error but no action? messed up something?)

Thanks!

Original comment by crazy4ch...@gmail.com on 8 Oct 2012 at 11:23

GoogleCodeExporter commented 9 years ago
Some more debugging done. I added optional debugging output. If you run into a 
problem, please set $debug=true; and try again, then post the output here.

Original comment by crazy4ch...@gmail.com on 10 Oct 2012 at 10:12

GoogleCodeExporter commented 9 years ago
Got an extrajob for a small web-catalogue. Will try your new version in action. 
You are doing a needful job. Thanks a lot.

PS : I wonder if i could just give phpliteadmin to a non-IT customer to 
add/remove items from his own website. Like phpmyadmin. But it's quite 
raw/buggy for doing that now.

Original comment by mas...@remort.net on 14 Oct 2012 at 8:07

GoogleCodeExporter commented 9 years ago
Just implemented ALTER TABLE RENAME TO for SQlite version2 databases in 
revision #244. 

You can get the latest development version from svn here:
http://phpliteadmin.googlecode.com/svn/source/1.9.3/phpliteadmin.php

Original comment by crazy4ch...@gmail.com on 25 Oct 2012 at 8:24

GoogleCodeExporter commented 9 years ago
This improvement definitely warrants a new release. Let me know when you get it 
to a stable enough state that we can include it in v1.9.3

Original comment by daneirac...@gmail.com on 26 Oct 2012 at 1:36

GoogleCodeExporter commented 9 years ago
Yes, I want to implement recreation of triggers and indexes and do some more 
testing and then we can release 1.9.3

Original comment by crazy4ch...@gmail.com on 26 Oct 2012 at 8:10

GoogleCodeExporter commented 9 years ago
Some more problems I still need to fix. I'll write them down here so I won't 
forget and you can track progress:

1. SQLite accepts any kind of non-SQL-standard quoting identifiers and values, 
so this is legal:
CREATE TABLE [my_table] ('corr''ect_sql' INTEGER, "wr""ong" TEXT, `wr``ong` 
TEXT, [wrong2] TEXT)
In sqlite_master, we get this original CREATE TABLE statement back, so my 
function currently would run into problems here.

Funny aside: SQLite does not reliably allow to use [wro]]ng]. I looked into 
SQLite's C-Code and found that sqlite3Dequote() does this correctly, but 
sqlite3GetToken() has an error that prevents this in at least some cases (Line 
111008 in current sqlite3.c amalgamation-version 3.7.14.1). Maybe I should open 
a bug at their bug tracker / send them a patch.

2. SQlite has even weaker typing than I expected. You can create any type 
on-the-fly like this:
CREATE TABLE my_table('a' my_new_type)
No need for
CREATE DISTINCT TYPE my_new_type AS TEXT;
or something like that.
Currently my function hard-codes possible types, so this would run into 
problems.
Easy to fix by getting the current datatype using PRAGMA table_info()

3. I implemented recreation of triggers & indexes, but this does not work yet 
with REANAME TO (sqlite2 only) because the table-name needs to be exchanges in 
the CREATE TRIGGER/INDEX statement.

So long for the moment.

Original comment by crazy4ch...@gmail.com on 27 Oct 2012 at 11:39

GoogleCodeExporter commented 9 years ago
i didn't get all the issues you've described here, but can remember some 
troubles with phpsqliteadmin when you've just come into this project. somewhere 
around 1.8 version. it should be actual in new version too. :

1. i've made a big bug about quotation. quotes in names of the records, as i 
expect, should work as good as in sqlite3 binary under linux and under the 
firefox-sqlite-manager-plugin. actually i don't think that weirdos like " 
'corr''ect_sql' INTEGER, " are realy needful for people. just compare quotation 
with mentioned tools. i still import huge cvs via sqlite3 and bash scripts to 
create initial sqlite db.

2. types. there are more types (~10) in official documentation, sqlite-manager 
from firefox. and may be in sqlite3 binary than in phpliteadmin. it may lead to 
inconcistance somehow... maybe... for example date(now) - doesn't work in 
phpsqliteadmin, but it works in sqlite3. it's generic.

3. in older versions i couldn't rename tables. didn't track it for new versions 
yet. may be it fixed...

thanks a lot for your work !

Original comment by mas...@remort.net on 28 Oct 2012 at 7:15

GoogleCodeExporter commented 9 years ago
Thanks for your feedback. I will also have a look at the firefox-plugin you 
mentioned.

1. Quotes in records work really fine in current svn version, no problem here 
any more. I am aware that probably (hopefully) not many users use special 
characters like quotes in their column- or table-names, but as sqlite permits 
them to do, I will support it.

2. You are not completely correct about the number of official types I think. 
See https://www.sqlite.org/datatype3.html
There are only 5 internal types ("storage classes") in SQlite3. Date for 
example can be one of INTEGER, TEXT or REAL, but is not really a special type 
of its own. You could use "my_date" instead of "DATE" and it would work the 
same way. But of course you are correct that DATE is used very often and lots 
of other tools treat it as a special type so we should support it as well. We 
will do so in future versions. Maybe we will also support custom type-names as 
well, although I don't really see the point of them. We have distinct types in 
SQL because they are distinct, i.e. we have strong typing. But SQlite does have 
very weak typing, so introducing a new type-name is more an annotation.

3. Renaming tables is supported by sqlite3 on its own. Problem is sqlite2 
doesn't support it, so this is why I implemented this in the alterTable 
function.

Original comment by crazy4ch...@gmail.com on 28 Oct 2012 at 12:26

GoogleCodeExporter commented 9 years ago
just assembled a new website. tried to use phpliteadmin only. it was 
unsuccessfull. there are some bugs with sqlite3 DB:
1. i still can't edit existing row parameters in a table.
a) it allows to reedit only name and type of the field (column). 
b) it doesn't do that actually. shows that all is done, but. nothing changed. 
achtung! ))
2. i can't create name of field like 'e-mail'. because of '-' symbol. but it's 
possible to create such a field in firefox-sqlite-manager.

check it right now (it's quick and easy to reproduce) and create/extend 
existing bugs if needed.

Anyway it gets better. Thanks.

Original comment by mas...@remort.net on 28 Oct 2012 at 1:02

GoogleCodeExporter commented 9 years ago
Thanks again for your feedback. Which version did you try? Please try the SVN 
version:
http://phpliteadmin.googlecode.com/svn/source/1.9.3/phpliteadmin.php

1.a) that's still true. We'll work on that in future versions.
1.b) should be fixed in SVN as long as you are not affected by one if the 
points I listed above
2. Please try this in the SVN version. Should work there.

Original comment by crazy4ch...@gmail.com on 28 Oct 2012 at 1:06

GoogleCodeExporter commented 9 years ago
it was 1.9.2 . thanks, i'll check SVN.

Original comment by mas...@remort.net on 28 Oct 2012 at 3:33

GoogleCodeExporter commented 9 years ago
SVN's 1.9.3 :
1. editing of fields works fine
2. i can create names of fields with '-' symbol

Big thanks for your work !

Original comment by mas...@remort.net on 28 Oct 2012 at 3:37

GoogleCodeExporter commented 9 years ago
I fixed the first 2 problems I mentioned in comment #30.

For 2., maybe it would be nice to let the user select the old type, but this is 
rather an enhancement than a defect. It now works as expected, replacing the 
old type with the new selected one.

Issue 3. from comment #30 is still to be done (recreating triggers/indexes for 
the correct table name after a rename).

Original comment by crazy4ch...@gmail.com on 28 Oct 2012 at 10:45

GoogleCodeExporter commented 9 years ago
Issue 116 has been merged into this issue.

Original comment by crazy4ch...@gmail.com on 30 Oct 2012 at 10:35

GoogleCodeExporter commented 9 years ago
*This comment continues with the numbering I started in comment #30 *

-- 3. --
I worked on the third issue mentioned in comment #30.

-- 3.a -- DONE
This means you can now rename tables in a SQLIte2-DB which has indexes assigned 
to it and it will keep (recreate) the indexes.

-- 3b. -- TODO
The same thing is still missing for recreation of triggers. I'll fix this soon. 
It's only that the CREATE TRIGGER statement is a little more complex than the 
CREATE INDEX statement and I just did not feel like writing the regex for this 
now ;-)
Coming soon...

-- 4a. -- TODO
I got another idea: When dropping or renaming a column with an index assigned, 
this won't work at the moment. I guess one would expect (or at least "hope"...) 
that the index is automatically dropped/adjusted as well in this case.
I'll implement this as well.

-- 4b. -- WON'T FIX
I guess the same thing is not easily implementable for triggers. Triggers can 
contain column-names in the WHEN-clause and in the statements between BEGIN and 
END as well. The ALTER TABLE statement might change these column-names so 
recreation will fail. But it would be _very_ hard to reliably replace changed 
column names in there. We'd need to build nearly a complete SQL parser. The 
column might even be referenced in a trigger of another table, so we'd need to 
check all triggers of all tables. I guess I'll stop at this point. I'm not even 
sure if other DBs that fully implement ALTER TABLE do this cleverly. I'd say 
it's no shame not to support this ;-)
Do other tools support this?

-- 5. -- TODO
I forgot about Foreign keys so far. I have not tried yet what happens if 
columns are dropped or renamed and there is a foreign key relationship to this 
column. I guess the relationship is being dropped, or the transaction fails.
I guess I'll need to have a look at this as well.
But as long as we do not really support foreign keys, maybe we can defer this 
to the next release (as long as the result of an ALTER TABLE does not break 
anything important here).

Original comment by crazy4ch...@gmail.com on 2 Nov 2012 at 8:46

GoogleCodeExporter commented 9 years ago
Issue 21 has been merged into this issue.

Original comment by crazy4ch...@gmail.com on 6 Nov 2012 at 3:22

GoogleCodeExporter commented 9 years ago
Issue #143 is an example where it did not work yet. I guess I'll merge #143 
into this one soon. I fixed the particular problem of #143 in svn and improved 
error handling. Now proper error messages are displayed if alter table fails.

Original comment by crazy4ch...@gmail.com on 13 Nov 2012 at 9:39

GoogleCodeExporter commented 9 years ago
Issue 143 has been merged into this issue.

Original comment by crazy4ch...@gmail.com on 1 Mar 2013 at 12:11

GoogleCodeExporter commented 9 years ago
Let's hope I get this closed completely in 1.9.5

Original comment by crazy4ch...@gmail.com on 19 Mar 2013 at 11:31

GoogleCodeExporter commented 9 years ago
Another concrete problem with ALTER TABLE has been discovered. See issue #202.

Original comment by crazy4ch...@gmail.com on 21 Mar 2013 at 11:24

GoogleCodeExporter commented 9 years ago
Issue #202 has been fixed.

As I lately set up the demo with the Chinook sample db, I noticed altering some 
of the tables in there also fails (without a proper error message, like issue 
#203). I guess mostly the use of foreign keys or indexes on these columns in 
there is the cause.
But before I close this issue here, I should make sure altering the chinook db 
tables works, or if not possible, at least gives a reasonable error message.

Original comment by crazy4ch...@gmail.com on 25 Mar 2013 at 10:51

GoogleCodeExporter commented 9 years ago
-- 4a. -- 
"When dropping or renaming a column with an index assigned,one would expect 
that the index is automatically dropped/adjusted as well in this case."

I just implemented this partly. In case of dropping a column, all indexes that 
refer to the column now get dropped implicitly.
Note that one can argue whether this is expected behaviour, but SQLite also 
implicitly drops indexes and triggers if you drop a table. So I'd say that this 
is in the spirit of SQLite.

In case of renaming a column with an index assigned, the query still will fail. 
I still need to implement this.

Original comment by crazy4ch...@gmail.com on 26 Apr 2013 at 10:55

GoogleCodeExporter commented 9 years ago
I think renaming a column should result in reflecting this change in indexes 
and triggers. Dropping them isn't in the spirit of making working with SQLite 
easier.

Original comment by roman...@gmail.com on 28 Apr 2013 at 12:22

GoogleCodeExporter commented 9 years ago
"I think renaming a column should result in reflecting this change in indexes 
and triggers."
Yes. For indexes, I will implement this for sure. But I guess for triggers this 
will get too complicated: In a CREATE TRIGGER statement, references to the 
column are between BEGIN and END inside SELECT, DELETE, UPDATE or INSERT 
statements. If you only imagine the complexity of parsing a SELECT statement, 
it is really tricky to replace column names in there correctly.

"Dropping them isn't in the spirit of making working with SQLite easier."
I am not sure what you mean here. Do you mean dropping indexes/triggers in case 
of a rename of a column or in case of dropping a column?
I think in case a column is renamed, I agree that triggers and indexes must not 
be dropped. I probably did not make it clear above. When I said "I still need 
to implement this", I meant that the change gets reflected in the index. 
Dropping would be very easy, but not a good solution.

In case a column is dropped, I think implicitly dropping indexes makes working 
with SQLite easier. If you want to get rid of a column, you don't care about 
the index. So it would be less easy if you get an error message and have to go 
back, drop the index yourself and then drop the column again.
As I said, when you drop a table, SQLite also implicitly drops everything 
assigned to it. So I think it is intuitive if the same happens if you drop a 
column with an index assigned.

Original comment by crazy4ch...@gmail.com on 28 Apr 2013 at 12:08

GoogleCodeExporter commented 9 years ago
Yes, you are right. I misunderstood you. Dropping a column should result in 
dropping indexes and triggers associated with it.

Regarding renaming a column... wouldn't simple match and replace be a good 
approach to reflecting column renames in triggers?

Original comment by roman...@gmail.com on 28 Apr 2013 at 3:03

GoogleCodeExporter commented 9 years ago
| Regarding renaming a column... wouldn't simple match and replace be a good 
approach 
| to reflecting column renames in triggers?

Unfortunately no. Your column name could occur within the CREATE TRIGGER 
statement at lots of places where it does not refer to your column but in a 
different context. The most simple example is something like this:
SELECT a.foo FROM a, b WHERE a.foo=b.foo
Now if you rename foo of table a into foo2 and do a simple search&replace, the 
statement would look like this:
SELECT a.foo2 FROM a, b WHERE a.foo2=b.foo2
But b.foo2 does not exist because it is still called b.
This is even an example that I consider quite realistic.

Moreover, you can have column names like "select" in SQLite. If you rename 
"select" to "bar", you would get:
bar a.foo FROM a, b WHERE a.foo=b.foo

I could give you hundreds of other examples where this would not work. It is 
far too dangerous to do a search & replace as things might happen that the user 
does not expect and does not even notice. As triggers really execute stuff, it 
is very dangerous to blindly mess around in there.

The only thing I could think of:
We give a message like:
"You rename a column and there is a trigger that seems to refer to that column 
name. You need to adjust the trigger to be able to complete the renameing of 
the column. Please correct the following CREATE TRIGGER statement:"
And then we will write the CREATE TRIGGER statement and everywhere the column 
name occurs, the user can choose (e.g. using a drop down) whether this should 
get replaced by the new column name or not.

For the user, this task should be comparably simple (at least if he knows SQL) 
whereas for phpLiteAdmin, it is rather tricky.

Original comment by crazy4ch...@gmail.com on 29 Apr 2013 at 9:46