sqlparse icon indicating copy to clipboard operation
sqlparse copied to clipboard

Two examples of subpar formatting results

Open pofl opened this issue 4 years ago • 2 comments

WITH all_customer_data AS (
 -- select all interesting data points out of customer metrics
 SELECT *
 FROM `data.customer_metrics`
 WHERE substage__c NOT IN ('Onboarding', 'Handover', 'Kickoff')
   AND carr_rollup__c >= 20000 
),
happiness_plugin AS (
 -- get happiness plugin data out of kibana metrics
 SELECT
   report_date AS h_report_date,
   branchID AS h_branchID,
   cnt_wau_happiness
 FROM `data.kibana_metrics`),
complete_data AS (
 -- join data sets
 SELECT *
 FROM all_customer_data
 JOIN happiness_plugin ON all_customer_data.report_date = h_report_date
 AND all_customer_data.branchid__c = h_branchID)
SELECT *
FROM complete_data

gets reformatted to

WITH all_customer_data AS
  (-- select all interesting data points out of customer metrics
 SELECT *
   FROM `data.customer_metrics`
   WHERE substage__c NOT IN ('Onboarding',
                             'Handover',
                             'Kickoff')
     AND carr_rollup__c >= 20000 ),
     happiness_plugin AS
  (-- get happiness plugin data out of kibana metrics
 SELECT report_date AS h_report_date,
        branchID AS h_branchID,
        cnt_wau_happiness
   FROM `data.kibana_metrics`),
     complete_data AS
  (-- join data sets
 SELECT *
   FROM all_customer_data
   JOIN happiness_plugin ON all_customer_data.report_date = h_report_date
   AND all_customer_data.branchid__c = h_branchID)
SELECT *
FROM complete_data

which is really off-putting:

  • comment after ( without even a space inbetween. IMO the comment should stay on an extra line.
  • ( misaligned with following SELECT

Also

WITH all_customer_data AS (
  SELECT *
  FROM `data.customer_metrics`
  -- throw away some uninteresting data
  WHERE substage__c NOT IN ('Onboarding', 'Handover', 'Kickoff')
  AND carr_rollup__c >= 20000
),
happiness_plugin_data_from_kibana AS (
  SELECT
    report_date,
    branchID,
    cnt_wau_happiness
  FROM `data.kibana_metrics`
)
SELECT *
FROM all_customer_data AS all
JOIN happiness_plugin_data_from_kibana AS happiness
ON all.report_date = happiness.report_date
AND all.branchid__c = happiness.branchID

becomes

WITH all_customer_data AS
  (SELECT *
   FROM `data.customer_metrics` -- throw away some uninteresting data

   WHERE substage__c NOT IN ('Onboarding',
                             'Handover',
                             'Kickoff')
     AND carr_rollup__c >= 20000 ),
     happiness_plugin_data_from_kibana AS
  (SELECT report_date,
          branchID,
          cnt_wau_happiness
   FROM `data.kibana_metrics`)
SELECT *
FROM all_customer_data AS ALL
JOIN happiness_plugin_data_from_kibana AS happiness ON all.report_date = happiness.report_date
AND all.branchid__c = happiness.branchID

which is much better than the first example. The result is definitely acceptable. And I know this is probably a matter of personal taste but I find it much less readable than my original version. However, that blank line there should definitely not happen.

pofl avatar Mar 09 '20 12:03 pofl

Thanks for providing those examples! I'll have to dig through the code.... pretty sure there was a change, that was related to the comment issue you're describing above. If there's only a comment in a line it should be preserved that way (as you described above).

andialbrecht avatar Mar 09 '20 12:03 andialbrecht

We've observed the same issue with a single line -- and multi-line comments /** .... */. Do you think this would warrant opening a new issue?

j-martin avatar Dec 03 '20 04:12 j-martin