Re: Redundant statements - Mailing list pgadmin-hackers

Le 29/04/2010 17:28, Erwin Brandstetter a écrit :
> On 29.04.2010 15:21, guillaume@lelarge.info wrote:
>> Sorry for not answering sooner.
>>
>
> No problem, the thread is old, but I just posted that yesterday. :)
>
>
>> Le 29/04/2010 14:15, Erwin Brandstetter a écrit :
>>
>>> On 28.04.2010 19:16, brandstetter@falter.at wrote:
>>>
>>>> Aloha!
>>>>
>>>> I am testing Guillaume's version of pgadmin3.exe from Apr. 17.
>>>>
>>>> The fix below is included and it seems to fix the problem just fine.
>>>> However, as a side effect (?) there is now a redundant statement for
>>>> each and every column:
>>>>      ALTER TABLE foo ALTER COLUMN bar SET STORAGE PLAIN;
>>>> or
>>>>      ALTER TABLE foo ALTER COLUMN baz SET STORAGE EXTENDED;
>>>>
>>>> I think those statements should only be included, if the storage type
>>>> differs from the default setting - like "SET STATISTICS" is handled
>>>> now.
>>>> It is pretty noisy as it is now.
>>>>
>> "SET STATISTICS" is not really handled this way. We had "SET STATISTICS"
>> statement if the value is bigger than zero. (Because if it is zero,
>> PostgreSQL will use the default_statistics_target setting.
>>
>
> Actually, 0 (zero) disables statistics gathering. -1 is for reverting to
> the system default.
> http://www.postgresql.org/docs/8.4/interactive/sql-altertable.html
>

Sorry, was wrong on this one :)

>> Anyway, I agree that "SET STORAGE" is really noisy. Suppose a table with
>> 20 columns. You will have 20 statements about storage. I agree that
>> something like this is needed. My problem is: what should we use as a
>> default value? PLAIN would be the default on non-toastable columns, and
>> EXTENDED the one to use with toastable columns? is there a way to be
>> sure of this?
>>
>>
>>>> On a side note: I would handle "WITH (OIDS=FALSE)" for table
>>>> definitions likewise: only print it, if it differs from the default
>>>> setting.
>>>>
>> If we do this, a user won't be able to copy the SQL and paste/execute it
>> on another server if this one has another value for default_with_oids.
>> So, I'm against its removal (unless someone could prove I'm wrong :) ).
>>
>
> As it is now, the policy is somewhat unclear. Some elements are added
> even though they are redundant under the local settings, others are not.
> If we could switch between complete and compact display, this would help
> to clear the situation.

+1

> In my daily work I reuse code in the same db cluster 99% of the time -
> 95% of the time in the same db. Redundant SQL is only cluttering the
> display and slows me down on a regular basis.
> One or the other redundant key word should not hurt, but we could strip
> most of the noise. (Reduces traffic a bit, too.)
> For cases where we want to copy objects to a different environment, the
> option "complete SQL" should be an improvement, too.
>
> I would define "local settings" as what "SHOW ALL" returns for the
> current connection (database / user defaults are in effect).
>

OK.

>>> Maybe an option "Show complete SQL"? To switch between "complete"
>>> (noisy) and default display (only what is necessary to create an
>>> identical object with the current settings).
>>> Personally I am only interested in the "compact" version, but other
>>> people's preferences may vary?
>>>
>> I'm not sure about this either. How could a user know about the
>> differences between a complete and a partial SQL?
>>
>
> We could have a note at the very top (possibly optional., too):
> -- SQL complete
> -- SQL for current connection
> Current settings are defined by the connection parameters displayed in
> the toolbar: host, user, database.
>

Seems better to me.

>>> This would be a major wishlist item. Not complicated, but possibly many
>>> lines of code.
>>>
>> Well, actually, the only thing I think is needed here is to display (or
>> not) some column's properties. Not complicated, not a lot of code.
>>
>
> Even better. :) But there are probably a number of cases, where we might
> want to add SQL to the "complete" version or trim in the "compact" version.
>

Sure. We'll need complete details on these cases :)

>>> What do you think of it? Should I create a ticket?
>>>
>> Well, I would like to have a better handling of the "SET STORAGE"
>> statement. So, yes, you can create a ticket on that, without being too
>> specific on the solution, which still needs some talk.
>>
>
> So we agree on "SET STORAGE". I'll create a ticket on that.
>

Yeah. I also found how to detect the "initial" value of the storage
(column typstorage in pg_catalog.pg_type). So I'll work on this right now.

> The option to switch between "complete" and "compact" is a different
> (more fundamental) feature. I would like it a lot, but I am not coding,
> so I can only suggest.
>

You can also create a ticket on this. But I'll refrain working on this,
waiting for Dave's opinion on this issue. And it probably won't be for
1.12, as beta will begin in a few days.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com