Skip to content

Commit 41de93c

Browse files
tglsfdctglsfdc
committed
Reduce indentation/parenthesization of set operations in rule/view dumps.
A query such as "SELECT x UNION SELECT y UNION SELECT z UNION ..." produces a left-deep nested parse tree, which we formerly showed in its full nested glory and with all the possible parentheses. This does little for readability, though, and long UNION lists resulting in excessive indentation are common. Instead, let's omit parentheses and indent all the subqueries at the same level in such cases. This patch skips indentation/parenthesization whenever the lefthand input of a SetOperationStmt is another SetOperationStmt of the same kind and ALL/DISTINCT property. We could teach the code the exact syntactic precedence of set operations and thereby avoid parenthesization in some more cases, but it's not clear that that'd be a readability win: it seems better to parenthesize if the set operation changes. (As an example, if there's one UNION in a long list of UNION ALL, it now stands out like a sore thumb, which seems like a good thing.) Back-patch to 9.3. This completes our response to a complaint from Greg Stark that since commit 62e6664 there's a performance problem in pg_dump for views containing long UNION sequences (or other types of deeply nested constructs). The previous commit 0601cb5 handles the general problem, but this one makes the specific case of UNION lists look a lot nicer.
1 parent 0601cb5 commit 41de93c

File tree

5 files changed

+390
-363
lines changed

5 files changed

+390
-363
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4714,42 +4714,59 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
47144714
else if (IsA(setOp, SetOperationStmt))
47154715
{
47164716
SetOperationStmt *op = (SetOperationStmt *) setOp;
4717-
4718-
if (PRETTY_INDENT(context))
4719-
{
4720-
context->indentLevel += PRETTYINDENT_STD;
4721-
appendStringInfoSpaces(buf, PRETTYINDENT_STD);
4722-
}
4717+
int subindent;
47234718

47244719
/*
4725-
* We force parens whenever nesting two SetOperationStmts. There are
4726-
* some cases in which parens are needed around a leaf query too, but
4727-
* those are more easily handled at the next level down (see code
4728-
* above).
4720+
* We force parens when nesting two SetOperationStmts, except when the
4721+
* lefthand input is another setop of the same kind. Syntactically,
4722+
* we could omit parens in rather more cases, but it seems best to use
4723+
* parens to flag cases where the setop operator changes. If we use
4724+
* parens, we also increase the indentation level for the child query.
4725+
*
4726+
* There are some cases in which parens are needed around a leaf query
4727+
* too, but those are more easily handled at the next level down (see
4728+
* code above).
47294729
*/
4730-
need_paren = !IsA(op->larg, RangeTblRef);
4730+
if (IsA(op->larg, SetOperationStmt))
4731+
{
4732+
SetOperationStmt *lop = (SetOperationStmt *) op->larg;
4733+
4734+
if (op->op == lop->op && op->all == lop->all)
4735+
need_paren = false;
4736+
else
4737+
need_paren = true;
4738+
}
4739+
else
4740+
need_paren = false;
47314741

47324742
if (need_paren)
4743+
{
47334744
appendStringInfoChar(buf, '(');
4745+
subindent = PRETTYINDENT_STD;
4746+
appendContextKeyword(context, "", subindent, 0, 0);
4747+
}
4748+
else
4749+
subindent = 0;
4750+
47344751
get_setop_query(op->larg, query, context, resultDesc);
4735-
if (need_paren)
4736-
appendStringInfoChar(buf, ')');
47374752

4738-
if (!PRETTY_INDENT(context))
4753+
if (need_paren)
4754+
appendContextKeyword(context, ") ", -subindent, 0, 0);
4755+
else if (PRETTY_INDENT(context))
4756+
appendContextKeyword(context, "", -subindent, 0, 0);
4757+
else
47394758
appendStringInfoChar(buf, ' ');
4759+
47404760
switch (op->op)
47414761
{
47424762
case SETOP_UNION:
4743-
appendContextKeyword(context, "UNION ",
4744-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4763+
appendStringInfoString(buf, "UNION ");
47454764
break;
47464765
case SETOP_INTERSECT:
4747-
appendContextKeyword(context, "INTERSECT ",
4748-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4766+
appendStringInfoString(buf, "INTERSECT ");
47494767
break;
47504768
case SETOP_EXCEPT:
4751-
appendContextKeyword(context, "EXCEPT ",
4752-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4769+
appendStringInfoString(buf, "EXCEPT ");
47534770
break;
47544771
default:
47554772
elog(ERROR, "unrecognized set op: %d",
@@ -4758,19 +4775,29 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
47584775
if (op->all)
47594776
appendStringInfoString(buf, "ALL ");
47604777

4761-
if (PRETTY_INDENT(context))
4762-
appendContextKeyword(context, "", 0, 0, 0);
4763-
4764-
need_paren = !IsA(op->rarg, RangeTblRef);
4778+
/* Always parenthesize if RHS is another setop */
4779+
need_paren = IsA(op->rarg, SetOperationStmt);
47654780

4781+
/*
4782+
* The indentation code here is deliberately a bit different from that
4783+
* for the lefthand input, because we want the line breaks in
4784+
* different places.
4785+
*/
47664786
if (need_paren)
4787+
{
47674788
appendStringInfoChar(buf, '(');
4789+
subindent = PRETTYINDENT_STD;
4790+
}
4791+
else
4792+
subindent = 0;
4793+
appendContextKeyword(context, "", subindent, 0, 0);
4794+
47684795
get_setop_query(op->rarg, query, context, resultDesc);
4769-
if (need_paren)
4770-
appendStringInfoChar(buf, ')');
47714796

47724797
if (PRETTY_INDENT(context))
4773-
context->indentLevel -= PRETTYINDENT_STD;
4798+
context->indentLevel -= subindent;
4799+
if (need_paren)
4800+
appendContextKeyword(context, ")", 0, 0, 0);
47744801
}
47754802
else
47764803
{

0 commit comments

Comments
 (0)