[Phpmyadmin-devel] Problems with Garvin's Patches

[robbat2 at orbis-terrarum.net]

On Tue, Feb 25, 2003 at 08:50:55AM +0100, Garvin Hicking wrote:
> > SELECT column_name, mimetype, transformation, transformation_options
> > FROM `column_comments`
> > WHERE db_name = 'my_database' AND table_name = 'my_table' AND (
> > mimetype != '' OR transformation != '' OR transformation_options != ''
> > )
> This query should only be used to build the table list in the left frame and for the
> strucutre (if 'display comments' is true) and for the headers/results of any query.
One bug to report, on the table create/alter pages the number of data
fields and the actual fields do NOT line up.

on the header row, after extra, there is:
Comments, Comments, MIME-type, Browser transformation, Transformation
options***

for the actual data rows:
text field, dropdown of auto-detect and mimetypes, dropdown of
transformations, text field, (missing cell).

This is just a little off by one error somewhere.

Looking at the HTML for that page, I see you are passing filenames for
the field_transformation variable. Take out the .inc.php3 part and go
and double check that code for security holes, as it may be susceptible
to data injection.

> I will add a code to "upgrade" the comment-table structure, if the fields are
> missing. Or is emitting a red warning on the start page a better way?
There should be a variable in the configuration file controlling if the
transform system should be used at all. If it is set to false, then it
does not get used. For the present moment, lets default it to false
while we work on the code. If the user activates it, look at showing an
error on the relations error page, and just a note elsewhere that they
should check that page.

I don't think it is right to allow the code to upgrade the comment-table
structure automatically. certainly we can have a script page that might
offer to do this if the PMA user has the correct privileges in it's
database, but we do not do it automatically. point the user to the
manual moreover. I believe the PMA user permissions that are mentioned
in the manual are only SELECT, INSERT, UPDATE, DELETE. no ALTER/CREATE
table there...

On that note, please update Documentation for your tranforms...

> > About you query box:
> > It appears a little large to me. Imho, we should only have the <textarea>
> > and the submit button there, nothing else.
> I can make the query history configurable, but I definitely want to have it.
> > It's just meant for quick queries, not for large imports. That is why I'd
> > like to drop the whole upload stuff. We still keep the SQL tab for this.
> Okay, I can talk about that. But as you can see, the query window only uses the
> snippet from tbl_query_box.php3, so that has to be modified as well.
Here is an idea for the quickbox, give it three mini-tabs:
"SQL", "Files", "History"
SQL - just the query box with the go and the field helper
Files - query from uploaded file and insert textfile into table
History - SQL history feature

> > The link "import textfile" and the table field list don't work at all, so
> > let's drop them, too.
> They don't work? I didn't test importing yet, but selecting fields worked for me
> once...
The field helper does work here, but ONLY if you start the quickbox from
a table page.

Garvin, a question + suggestion:
does the SQL history work thru links only? 
What about doing it via a new table in the PMA database instead,
configurable properly.  This is after it gets moved to a seperate tab.
it should have a configuration option to only display the N most recent
items.
suggested table layout:
id - longint auto_increment primary key
username - varchar(64) index
db - varchar(64) index
timevalue - timestamp index
sqlquery - varchar(255) should work for short queries, but we may want
one of the TEXT types instead (i think 64k is a better limit)

behaviour:
on login and logout or a large number of rows existing, run a query on
the table that deletes ALL but the most N recent items for the current
user+database pair. also have an explict button on the SQL history page
that can clean up the table.

To cater for as many usergroups as possible, there should be yet another
config option to allow the admin to set the code to use either the
current SQL history via links or the new idea via an SQL table, or
disable it totally. (Do NOT use the value of the tablename for this
behaviour).

> > Keep on the good work,
> Thanks! I appreciate that a lot :)
Same from this side, it all looks like it will be really good once the
bugs are worked out.

-- 
Robin Hugh Johnson
E-Mail     : robbat2 at orbis-terrarum.net
Home Page  : http://www.orbis-terrarum.net/?l=people.robbat2
ICQ#       : 30269588 or 41961639
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20030225/ec255ec1/attachment.sig>