Testing 1.7: issues in edit grid & query window

Hi developers! Hi Dave!

Still testing pgAdmin III 1.7.0, rev 6292:6293. Client Win XP, hosts:
Debian Sarge / PG 8.1.8 (and occasionally Debian Etch / PG 8.2.3)

Edit grid
=========

The edit grid suppresses NULL values ON INSERTs (UPDATEs are not
affected). Example. Say we have a table:

CREATE TABLE mankind
(
man_id integer primary key,
people_id integer,
king boolean DEFAULT false
);

When I enter "24 for "man_id", "2" for "people_id" and do not enter
anything for "king" in the edit grid this statement is sent to the database:
INSERT INTO mankind(man_id, people_id) VALUES ('24'::integer,
'2'::integer)
Which is correct.

When I do the same, but explicitly select NULL in the control for the
boolean field king, though, the statement will still be the same:
INSERT INTO mankind(man_id, people_id) VALUES ('24'::integer,
'2'::integer)
Which is wrong.
The field "king" will be assigned the DEFAULT VALUE false.

It should be
INSERT INTO mankind(man_id, people_id, king) VALUES ('24'::integer,
'2'::integer, 'NULL'::boolean)
So the field is actually set to NULL.

In contrast to that, UPDATEs work as expected:
UPDATE mankind SET king=NULL::boolean WHERE man_id = '24'::integer

While being at it, I noticed a somewhat inconsistent formatting:
INSERT INTO mankind(man_id, people_id, king) VALUES ('24'::integer,
'2'::integer, TRUE::boolean)
These forms would be consistent:
INSERT INTO mankind(man_id, people_id, king) VALUES ('24'::integer,
'2'::integer, 'TRUE'::boolean)
INSERT INTO mankind(man_id, people_id, king) VALUES (24::integer,
2::integer, TRUE::boolean)
INSERT INTO mankind(man_id, people_id, king) VALUES (24, 2, TRUE)
You get the point - why treat boolean differently?
All of the above is present in 1.6.3.

Query Window
============

If I have a couple of windows open and get an error message in one of
them and do not click it away (because I want to keep the text while
checking on the issue), then the following weirdness occurs:
Pressing <Ctrl><c> does not copy selected text in the query window any
more, but copies the text of the open error message of the other window
(somewhere in the background).
Copying via selecting "Copy" from the context menu will copy the
selected text as expected.
This is also present in 1.6.3.

-----

If I select text and type something else the selected text will be
replaced. This works as it should.
When I press <Enter>, however, the select text is NOT replaced. Instead
the line-break is inserted at the end of the selected text. Might share
roots with the issue below.
This is NOT present in 1.6.3, where it works as expected.

-----

You have a new auto-indentation in 1.7, which works fine most of the
time. However, sometimes it fails with existing lines of text. Try the
following test case:
Both lines are indented the same (three spaces in my example):
---
text1
text2
---

Now, place the cursor right before the first printable character and
press <Enter>. It works as expected, mostly:
---
text1

text2
---
Note, however, that useless whitespace remains on the empty line in between.

If you place the cursor at the beginning of the line and press <Enter>,
the leading whitespace is doubled (which is not as expected)
---
text1

text2
---
At least, the whitespace in between is gone.
Obviously, this is not present in 1.6.3.

That's all, folks! For now. :)

Regards
Erwin

By the way, the text representation at archives.postgresql.org
completely messes up the format. So you can't see what I am trying to to
demonstrate in indentation examples.
Happens here:
http://archives.postgresql.org/pgadmin-hackers/2007-06/msg00005.php

Regards
Erwin

Erwin Brandstetter wrote:
> Hi developers! Hi Dave!

Hi Erwin,

Edit grid first - will follow up later with the others (it's easier for
me to deal with each distinct subject in separate messages)

> Edit grid
> =========
>
> The edit grid suppresses NULL values ON INSERTs (UPDATEs are not
> affected). Example. Say we have a table:
>
> CREATE TABLE mankind
> (
> man_id integer primary key,
> people_id integer,
> king boolean DEFAULT false
> );
>
> When I enter "24 for "man_id", "2" for "people_id" and do not enter
> anything for "king" in the edit grid this statement is sent to the
> database:
> INSERT INTO mankind(man_id, people_id) VALUES ('24'::integer,
> '2'::integer)
> Which is correct.
>
> When I do the same, but explicitly select NULL in the control for the
> boolean field king, though, the statement will still be the same:
> INSERT INTO mankind(man_id, people_id) VALUES ('24'::integer,
> '2'::integer)
> Which is wrong.

After figuring out why, and modifying it to work as you suggest, it
strikes me that actually it's not wrong imho. In all cases, the edit
grid currently takes null-on-insert to mean 'don't insert this value'.
This ensures that default values for the column will always be respected
upon insert - for example, leave a text column empty, and it'll pick up
it's default value. If we made empty/blank/undetermined consistently
force the value to NULL, there would be no way to pickup the default
value for a column. As it is now, the worst case is a 2nd edit is
required to reset a column with a default value back to null.

It is possible to change this of course by having a quad state checkbox
for booleans, and allowing a keyword such as <DEFAULT> to be entered
into other types. There are a couple of problems with that, not least of
which is that we'd need to create a custom checkbox control because we
only have tri state ones at present. We'd also need to figure out some
way to actually enter <DEFAULT> into a text column in case that was a
legitimate data value for some people, and to alllow those characters in
a numeric column, but only in that order, and on their own.

I'm not sure this is ever likely to get fixed to be honest.

Regards, Dave.

Erwin Brandstetter wrote:

Hi Erwin,

> Query Window
> ============
>
> If I have a couple of windows open and get an error message in one of
> them and do not click it away (because I want to keep the text while
> checking on the issue), then the following weirdness occurs:
> Pressing <Ctrl><c> does not copy selected text in the query window any
> more, but copies the text of the open error message of the other window
> (somewhere in the background).
> Copying via selecting "Copy" from the context menu will copy the
> selected text as expected.
> This is also present in 1.6.3.

Not much I can do about that - the message box catches the ctrl+c.

> -----
>
> If I select text and type something else the selected text will be
> replaced. This works as it should.
> When I press <Enter>, however, the select text is NOT replaced. Instead
> the line-break is inserted at the end of the selected text. Might share
> roots with the issue below.
> This is NOT present in 1.6.3, where it works as expected.

Fixed, and yes, it was because of the auto indent.

>
> You have a new auto-indentation in 1.7, which works fine most of the
> time. However, sometimes it fails with existing lines of text. Try the
> following test case:
> Both lines are indented the same (three spaces in my example):
> ---
> text1
> text2
> ---
>
> Now, place the cursor right before the first printable character and
> press <Enter>. It works as expected, mostly:
> ---
> text1
>
> text2
> ---
> Note, however, that useless whitespace remains on the empty line in
> between.

We now clear the whitespace on the previous line.

> If you place the cursor at the beginning of the line and press <Enter>,
> the leading whitespace is doubled (which is not as expected)
> ---
> text1
>
> text2
> ---
> At least, the whitespace in between is gone.
> Obviously, this is not present in 1.6.3.

We no longer duplicate the indent.

Thanks for the report - I'll produce a new build to test sometime soon.

Regards, Dave

Hi Dave!

dpage(at)postgresql(dot)org wrote:
(...)
> After figuring out why, and modifying it to work as you suggest, it
> strikes me that actually it's not wrong imho. In all cases, the edit
> grid currently takes null-on-insert to mean 'don't insert this value'.
> This ensures that default values for the column will always be respected
> upon insert - for example, leave a text column empty, and it'll pick up
> it's default value. If we made empty/blank/undetermined consistently
> force the value to NULL, there would be no way to pickup the default
> value for a column. As it is now, the worst case is a 2nd edit is
> required to reset a column with a default value back to null.
>
> It is possible to change this of course by having a quad state checkbox
> for booleans, and allowing a keyword such as <DEFAULT> to be entered
> into other types. There are a couple of problems with that, not least of
> which is that we'd need to create a custom checkbox control because we
> only have tri state ones at present. We'd also need to figure out some
> way to actually enter <DEFAULT> into a text column in case that was a
> legitimate data value for some people, and to alllow those characters in
> a numeric column, but only in that order, and on their own.
>
> I'm not sure this is ever likely to get fixed to be honest.

So this could be fixed for boolean values but not so easily for others.
I propose a TODO list item:

- EditGrid: distinguish between entering NULL and nothing (= DEFAULT).

Regards
Erwin

Hi Dave!

Contrary to what I thought I had observed and what I announced in my
reply to your new snapshot (partly fixed), I cannot reproduce any of the
problems in rev. 6357 any more, sorry. (Or am I?)
Everything auto-indentation works like a charm, as far as further tests
have shown. Thanks and sorry for the noise.

Regards
Erwin

Erwin Brandstetter wrote:
> Hi Dave!
>
> Contrary to what I thought I had observed and what I announced in my
> reply to your new snapshot (partly fixed), I cannot reproduce any of
> the problems in rev. 6357 any more, sorry. (Or am I?)
> Everything auto-indentation works like a charm, as far as further
> tests have shown. Thanks and sorry for the noise.

Well, well. After further testing I can shed some light on this. The
multybyte beast rears its ugly head again
The problems ARE still there, with any multybyte characters prior to the
cursor. That's why I could observe them in one test and could not
reproduce them in the next.
The issue with selected text not being replaced by <enter> is gone for
good, however.

Also, auto-indentation still gets confused if you enter the cursor
somewhere in the middle of the non-printing indentation characters and
press <enter>. In this case indentation space is added up and blanks on
previous lines are left behind.

Regards
Erwin

Erwin Brandstetter wrote:
> Erwin Brandstetter wrote:
>> Hi Dave!
>>
>> Contrary to what I thought I had observed and what I announced in my
>> reply to your new snapshot (partly fixed), I cannot reproduce any of
>> the problems in rev. 6357 any more, sorry. (Or am I?)
>> Everything auto-indentation works like a charm, as far as further
>> tests have shown. Thanks and sorry for the noise.
>
> Well, well. After further testing I can shed some light on this. The
> multybyte beast rears its ugly head again
> The problems ARE still there, with any multybyte characters prior to the
> cursor. That's why I could observe them in one test and could not
> reproduce them in the next.

Hmm, yes - not much I can do about that without a fix for wxWidgets :-(

> The issue with selected text not being replaced by <enter> is gone for
> good, however.

:-)

> Also, auto-indentation still gets confused if you enter the cursor
> somewhere in the middle of the non-printing indentation characters and
> press <enter>. In this case indentation space is added up and blanks on
> previous lines are left behind.

OK, think I've got this nailed. I'll see about a new snapshot soon.

Thanks, Dave