Skip to content

Commit 0652d77

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 de6c439 commit 0652d77

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
@@ -4581,42 +4581,59 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
45814581
else if (IsA(setOp, SetOperationStmt))
45824582
{
45834583
SetOperationStmt *op = (SetOperationStmt *) setOp;
4584-
4585-
if (PRETTY_INDENT(context))
4586-
{
4587-
context->indentLevel += PRETTYINDENT_STD;
4588-
appendStringInfoSpaces(buf, PRETTYINDENT_STD);
4589-
}
4584+
int subindent;
45904585

45914586
/*
4592-
* We force parens whenever nesting two SetOperationStmts. There are
4593-
* some cases in which parens are needed around a leaf query too, but
4594-
* those are more easily handled at the next level down (see code
4595-
* above).
4587+
* We force parens when nesting two SetOperationStmts, except when the
4588+
* lefthand input is another setop of the same kind. Syntactically,
4589+
* we could omit parens in rather more cases, but it seems best to use
4590+
* parens to flag cases where the setop operator changes. If we use
4591+
* parens, we also increase the indentation level for the child query.
4592+
*
4593+
* There are some cases in which parens are needed around a leaf query
4594+
* too, but those are more easily handled at the next level down (see
4595+
* code above).
45964596
*/
4597-
need_paren = !IsA(op->larg, RangeTblRef);
4597+
if (IsA(op->larg, SetOperationStmt))
4598+
{
4599+
SetOperationStmt *lop = (SetOperationStmt *) op->larg;
4600+
4601+
if (op->op == lop->op && op->all == lop->all)
4602+
need_paren = false;
4603+
else
4604+
need_paren = true;
4605+
}
4606+
else
4607+
need_paren = false;
45984608

45994609
if (need_paren)
4610+
{
46004611
appendStringInfoChar(buf, '(');
4612+
subindent = PRETTYINDENT_STD;
4613+
appendContextKeyword(context, "", subindent, 0, 0);
4614+
}
4615+
else
4616+
subindent = 0;
4617+
46014618
get_setop_query(op->larg, query, context, resultDesc);
4602-
if (need_paren)
4603-
appendStringInfoChar(buf, ')');
46044619

4605-
if (!PRETTY_INDENT(context))
4620+
if (need_paren)
4621+
appendContextKeyword(context, ") ", -subindent, 0, 0);
4622+
else if (PRETTY_INDENT(context))
4623+
appendContextKeyword(context, "", -subindent, 0, 0);
4624+
else
46064625
appendStringInfoChar(buf, ' ');
4626+
46074627
switch (op->op)
46084628
{
46094629
case SETOP_UNION:
4610-
appendContextKeyword(context, "UNION ",
4611-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4630+
appendStringInfoString(buf, "UNION ");
46124631
break;
46134632
case SETOP_INTERSECT:
4614-
appendContextKeyword(context, "INTERSECT ",
4615-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4633+
appendStringInfoString(buf, "INTERSECT ");
46164634
break;
46174635
case SETOP_EXCEPT:
4618-
appendContextKeyword(context, "EXCEPT ",
4619-
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
4636+
appendStringInfoString(buf, "EXCEPT ");
46204637
break;
46214638
default:
46224639
elog(ERROR, "unrecognized set op: %d",
@@ -4625,19 +4642,29 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
46254642
if (op->all)
46264643
appendStringInfo(buf, "ALL ");
46274644

4628-
if (PRETTY_INDENT(context))
4629-
appendContextKeyword(context, "", 0, 0, 0);
4630-
4631-
need_paren = !IsA(op->rarg, RangeTblRef);
4645+
/* Always parenthesize if RHS is another setop */
4646+
need_paren = IsA(op->rarg, SetOperationStmt);
46324647

4648+
/*
4649+
* The indentation code here is deliberately a bit different from that
4650+
* for the lefthand input, because we want the line breaks in
4651+
* different places.
4652+
*/
46334653
if (need_paren)
4654+
{
46344655
appendStringInfoChar(buf, '(');
4656+
subindent = PRETTYINDENT_STD;
4657+
}
4658+
else
4659+
subindent = 0;
4660+
appendContextKeyword(context, "", subindent, 0, 0);
4661+
46354662
get_setop_query(op->rarg, query, context, resultDesc);
4636-
if (need_paren)
4637-
appendStringInfoChar(buf, ')');
46384663

46394664
if (PRETTY_INDENT(context))
4640-
context->indentLevel -= PRETTYINDENT_STD;
4665+
context->indentLevel -= subindent;
4666+
if (need_paren)
4667+
appendContextKeyword(context, ")", 0, 0, 0);
46414668
}
46424669
else
46434670
{

0 commit comments

Comments
 (0)