SEO

[Please use the list!]

> -----Original Message-----
> From: Edward Di Geronimo Jr. [mailto:edigeronimo@xtracards.com]
> Sent: 08 June 2006 00:55
> To: Dave Page
> Subject: SQL Code Formatting Patch
>
> Hi Dave,
>
> I got sick of the unreadable SQL displayed as the source of
> views, so
> I redid the formatting code. Complex views are very nice to
> read now.
> Below is a sample before and after, using the view that
> prompted me to
> do this. Attached is the patch.

OK, looks quite nice... But (you knew there was a but right?) the brace
positioning around sub selects seems a little off, eg:

------------------------------------
CREATE OR REPLACE VIEW rep_suppreqs AS

SELECT sr_header.sr_guid,
sr_header.sr_timestamp,
sr_header.sr_owner,
sr_header.sr_user,
sr_header.sr_email,
sr_header.sr_phone,
sr_header.sr_fax,
sr_header.sr_hardware_id,
sr_header.sr_software_id,
sr_header.sr_status,
sr_header.sr_type,
sr_header.sr_helpdesk,
sr_header.sr_helpdesk_id,
sr_header.sr_helpdesk_timestamp,
sr_header.sr_description,
sr_header.sr_details,
sr_header.sr_priority,
sr_header.sr_updated,
sr_header.sr_owner_status,

CASE
WHEN sr_header.sr_status::text = 'U'::character
varying::text THEN 'With User'::text
WHEN sr_header.sr_status::text = 'O'::character
varying::text THEN 'Open'::text
WHEN sr_header.sr_status::text = 'H'::character
varying::text THEN 'With Helpdesk'::text
WHEN sr_header.sr_status::text = 'C'::character
varying::text THEN 'Closed'::text
ELSE '** UNKNOWN **'::text
END AS status,

CASE
WHEN sr_header.sr_type::text = 'H'::character varying::text
THEN 'Hardware Fault'::text
WHEN sr_header.sr_type::text = 'S'::character varying::text
THEN 'Software Fault'::text
WHEN sr_header.sr_type::text = 'I'::character varying::text
THEN 'Information Request'::text
WHEN sr_header.sr_type::text = 'W'::character varying::text
THEN 'Work Request'::text
WHEN sr_header.sr_type::text = 'B'::character varying::text
THEN 'Beta Test Report'::text
ELSE '** UNKNOWN **'::text
END AS "type",
(
SELECT sys_user.fullname
FROM sys_user
WHERE sys_user.username::text = sr_header.sr_owner::text)
AS "owner",
(
SELECT (((a1.gbl_name::text || ' - '::character
varying::text) || s1.sw_name::text) || ' v'::character varying::text) ||
s1.sw_version::text
FROM sw_software s1,
gbl_addr_book a1
WHERE s1.sw_vendor = a1.gbl_guid
AND s1.sw_guid = sr_header.sr_software_id) AS
software,
(
SELECT ((((h2.hd_hardware_id::text || ' ('::character
varying::text) || a2.gbl_name::text) || ',
'::character varying::text) ||
h2.hd_model::text) || ')'::character varying::text
FROM hd_hardware h2,
gbl_addr_book a2
WHERE h2.hd_manufacturer = a2.gbl_guid
AND h2.hd_guid = sr_header.sr_hardware_id) AS
hardware,
(
SELECT a3.gbl_name
FROM gbl_addr_book a3
WHERE a3.gbl_guid = sr_header.sr_helpdesk) AS helpdesk

FROM sr_header;
------------------------------------

I would expect the subselects to look more like:

(
SELECT ((((h2.hd_hardware_id::text || ' ('::character
varying::text) || a2.gbl_name::text) || ',
'::character varying::text) ||
h2.hd_model::text) || ')'::character varying::text
FROM hd_hardware h2,
gbl_addr_book a2
WHERE h2.hd_manufacturer = a2.gbl_guid
AND h2.hd_guid = sr_header.sr_hardware_id
) AS hardware,

Having the braces in non-matched positions decreases the readability I
think. Thoughts?

> Hope you had a nice trip to Paris.

Yes, very good thanks :-)

Regards. Dave

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Quoting Dave Page <dpage@vale-housing.co.uk>:

> I would expect the subselects to look more like:
>
> (
> SELECT ((((h2.hd_hardware_id::text || ' ('::character
> varying::text) || a2.gbl_name::text) || ',
> '::character varying::text) ||
> h2.hd_model::text) || ')'::character varying::text
> FROM hd_hardware h2,
> gbl_addr_book a2
> WHERE h2.hd_manufacturer = a2.gbl_guid
> AND h2.hd_guid = sr_header.sr_hardware_id
> ) AS hardware,
>
> Having the braces in non-matched positions decreases the readability I
> think. Thoughts?

I agree that it would be better as you wrote, and that was my original
intention, but I'm having trouble pulling that off without side
effects that make things look much worse. I've got a version that
handles the parens around the select ok, but, has issues with the
other parens in that example. The above would come out something like
this:

(
SELECT ((((h2.hd_hardware_id::text || ' ('::character
varying::text
) || a2.gbl_name::text
) || ', '::character varying::text
) || h2.hd_model::text
) || ')'::character varying::text
FROM hd_hardware h2,
gbl_addr_book a2
WHERE h2.hd_manufacturer = a2.gbl_guid
AND h2.hd_guid = sr_header.sr_hardware_id
) AS hardware,

I'll look at it a little more, but I don't have high hopes of getting
it better without making the scanner try to understand the SQL instead
of the current approach of just reacting to things of interest. I do
have an idea that may work, but I'm not sure yet. It'll take a while
for me to work out, and I'm not sure how quickly I'll be able to get
to it, so you may want to go ahead with the current patch for now.

BTW, there's a table of keywords in the code that defines what should
be done when they're detected. There's a bool at the end that
determines if a new line should be inserted before the keyword. You
may want to change that to false for the CASE keyword. I wasn't sure
if there were instances where that would backfire, so I figured extra
newlines were better than not enough of them. My main concern is if
you try using case somewhere in the where clause.

Ed

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org