Skip to content

Commit aef7a0c

Browse files
tglsfdctglsfdc
committed
Parse JOIN/ON conditions with the proper visibility of input columns,
ie, consider only the columns coming from the JOIN clause's sub-clauses. Also detect attempts to reference columns belonging to other tables (which would still be possible using an explicitly-qualified name). I'm not sure this implements the spec's semantics 100% accurately, but at least it gives plausible behavior.
1 parent 418bd67 commit aef7a0c

File tree

1 file changed

+115
-30
lines changed

1 file changed

+115
-30
lines changed

‎src/backend/parser/parse_clause.c

Lines changed: 115 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.66 2000/09/12 21:07:02 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.67 2000/09/17 22:21:27 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515

1616
#include "postgres.h"
1717

1818
#include "access/heapam.h"
19-
#include "optimizer/tlist.h"
2019
#include "nodes/makefuncs.h"
20+
#include "optimizer/tlist.h"
21+
#include "optimizer/var.h"
2122
#include "parser/analyze.h"
2223
#include "parser/parse.h"
2324
#include "parser/parsetree.h"
@@ -38,12 +39,15 @@ static char *clauseText[] = {"ORDER BY", "GROUP BY", "DISTINCT ON"};
3839
static void extractUniqueColumns(List *common_colnames,
3940
List *src_colnames, List *src_colvars,
4041
List **res_colnames, List **res_colvars);
41-
static Node *transformUsingClause(ParseState *pstate,
42-
List *leftVars, List *rightVars);
42+
static Node *transformJoinUsingClause(ParseState *pstate,
43+
List *leftVars, List *rightVars);
44+
static Node *transformJoinOnClause(ParseState *pstate, JoinExpr *j,
45+
List *containedRels);
4346
static RangeTblRef *transformTableEntry(ParseState *pstate, RangeVar *r);
4447
static RangeTblRef *transformRangeSubselect(ParseState *pstate,
4548
RangeSubselect *r);
46-
static Node *transformFromClauseItem(ParseState *pstate, Node *n);
49+
static Node *transformFromClauseItem(ParseState *pstate, Node *n,
50+
List **containedRels);
4751
static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node,
4852
List *tlist, int clause);
4953
static List *addTargetToSortList(TargetEntry *tle, List *sortlist,
@@ -76,8 +80,9 @@ makeRangeTable(ParseState *pstate, List *frmList)
7680
foreach(fl, frmList)
7781
{
7882
Node *n = lfirst(fl);
83+
List *containedRels;
7984

80-
n = transformFromClauseItem(pstate, n);
85+
n = transformFromClauseItem(pstate, n, &containedRels);
8186
pstate->p_jointree = lappend(pstate->p_jointree, n);
8287
}
8388
}
@@ -164,13 +169,13 @@ extractUniqueColumns(List *common_colnames,
164169
*res_colvars = new_colvars;
165170
}
166171

167-
/* transformUsingClause()
168-
* Build a complete ON clause from a partially-transformed USING list.
169-
* We are given lists of Var nodes representing left and right match columns.
170-
* Result is a transformed qualification expression.
172+
/* transformJoinUsingClause()
173+
* Build a complete ON clause from a partially-transformed USING list.
174+
* We are given lists of nodes representing left and right match columns.
175+
* Result is a transformed qualification expression.
171176
*/
172177
static Node *
173-
transformUsingClause(ParseState *pstate, List *leftVars, List *rightVars)
178+
transformJoinUsingClause(ParseState *pstate, List *leftVars, List *rightVars)
174179
{
175180
Node *result = NULL;
176181
List *lvars,
@@ -210,18 +215,82 @@ transformUsingClause(ParseState *pstate, List *leftVars, List *rightVars)
210215
rvars = lnext(rvars);
211216
}
212217

218+
/*
219+
* Since the references are already Vars, and are certainly from the
220+
* input relations, we don't have to go through the same pushups that
221+
* transformJoinOnClause() does. Just invoke transformExpr() to fix
222+
* up the operators, and we're done.
223+
*/
213224
result = transformExpr(pstate, result, EXPR_COLUMN_FIRST);
214225

215226
if (exprType(result) != BOOLOID)
216227
{
217228
/* This could only happen if someone defines a funny version of '=' */
218-
elog(ERROR, "USING clause must return type bool, not type %s",
229+
elog(ERROR, "JOIN/USING clause must return type bool, not type %s",
219230
typeidTypeName(exprType(result)));
220231
}
221232

222233
return result;
223-
} /* transformUsingClause() */
234+
} /* transformJoinUsingClause() */
235+
236+
/* transformJoinOnClause()
237+
* Transform the qual conditions for JOIN/ON.
238+
* Result is a transformed qualification expression.
239+
*/
240+
static Node *
241+
transformJoinOnClause(ParseState *pstate, JoinExpr *j,
242+
List *containedRels)
243+
{
244+
Node *result;
245+
List *sv_jointree;
246+
List *clause_varnos,
247+
*l;
248+
249+
/*
250+
* This is a tad tricky, for two reasons. First, at the point where
251+
* we're called, the two subtrees of the JOIN node aren't yet part of
252+
* the pstate's jointree, which means that transformExpr() won't resolve
253+
* unqualified references to their columns correctly. We fix this in a
254+
* slightly klugy way: temporarily make the pstate's jointree consist of
255+
* just those two subtrees (which creates exactly the namespace the ON
256+
* clause should see). This is OK only because the ON clause can't
257+
* legally alter the jointree by causing relation refs to be added.
258+
*/
259+
sv_jointree = pstate->p_jointree;
260+
pstate->p_jointree = lcons(j->larg, lcons(j->rarg, NIL));
261+
262+
/* This part is just like transformWhereClause() */
263+
result = transformExpr(pstate, j->quals, EXPR_COLUMN_FIRST);
264+
if (exprType(result) != BOOLOID)
265+
{
266+
elog(ERROR, "JOIN/ON clause must return type bool, not type %s",
267+
typeidTypeName(exprType(result)));
268+
}
269+
270+
pstate->p_jointree = sv_jointree;
271+
272+
/*
273+
* Second, we need to check that the ON condition doesn't refer to any
274+
* rels outside the input subtrees of the JOIN. It could do that despite
275+
* our hack on the jointree if it uses fully-qualified names. So, grovel
276+
* through the transformed clause and make sure there are no bogus
277+
* references.
278+
*/
279+
clause_varnos = pull_varnos(result);
280+
foreach(l, clause_varnos)
281+
{
282+
int varno = lfirsti(l);
224283

284+
if (! intMember(varno, containedRels))
285+
{
286+
elog(ERROR, "JOIN/ON clause refers to \"%s\", which is not part of JOIN",
287+
rt_fetch(varno, pstate->p_rtable)->eref->relname);
288+
}
289+
}
290+
freeList(clause_varnos);
291+
292+
return result;
293+
}
225294

226295
/*
227296
* transformTableEntry --- transform a RangeVar (simple relation reference)
@@ -309,25 +378,40 @@ transformRangeSubselect(ParseState *pstate, RangeSubselect *r)
309378
* range table list being built in the ParseState, and return the
310379
* transformed item ready to include in the jointree list.
311380
* This routine can recurse to handle SQL92 JOIN expressions.
381+
*
382+
* Aside from the primary return value (the transformed jointree item)
383+
* this routine also returns an integer list of the rangetable indexes
384+
* of all the base relations represented in the jointree item. This
385+
* list is needed for checking JOIN/ON conditions in higher levels.
312386
*/
313387
static Node *
314-
transformFromClauseItem(ParseState *pstate, Node *n)
388+
transformFromClauseItem(ParseState *pstate, Node *n, List **containedRels)
315389
{
316390
if (IsA(n, RangeVar))
317391
{
318392
/* Plain relation reference */
319-
return (Node *) transformTableEntry(pstate, (RangeVar *) n);
393+
RangeTblRef *rtr;
394+
395+
rtr = transformTableEntry(pstate, (RangeVar *) n);
396+
*containedRels = lconsi(rtr->rtindex, NIL);
397+
return (Node *) rtr;
320398
}
321399
else if (IsA(n, RangeSubselect))
322400
{
323-
/* Plain relation reference */
324-
return (Node *) transformRangeSubselect(pstate, (RangeSubselect *) n);
401+
/* sub-SELECT is like a plain relation */
402+
RangeTblRef *rtr;
403+
404+
rtr = transformRangeSubselect(pstate, (RangeSubselect *) n);
405+
*containedRels = lconsi(rtr->rtindex, NIL);
406+
return (Node *) rtr;
325407
}
326408
else if (IsA(n, JoinExpr))
327409
{
328410
/* A newfangled join expression */
329411
JoinExpr *j = (JoinExpr *) n;
330-
List *l_colnames,
412+
List *l_containedRels,
413+
*r_containedRels,
414+
*l_colnames,
331415
*r_colnames,
332416
*res_colnames,
333417
*l_colvars,
@@ -337,8 +421,13 @@ transformFromClauseItem(ParseState *pstate, Node *n)
337421
/*
338422
* Recursively process the left and right subtrees
339423
*/
340-
j->larg = transformFromClauseItem(pstate, j->larg);
341-
j->rarg = transformFromClauseItem(pstate, j->rarg);
424+
j->larg = transformFromClauseItem(pstate, j->larg, &l_containedRels);
425+
j->rarg = transformFromClauseItem(pstate, j->rarg, &r_containedRels);
426+
427+
/*
428+
* Generate combined list of relation indexes
429+
*/
430+
*containedRels = nconc(l_containedRels, r_containedRels);
342431

343432
/*
344433
* Extract column name and var lists from both subtrees
@@ -463,7 +552,7 @@ transformFromClauseItem(ParseState *pstate, Node *n)
463552
ndx++;
464553
}
465554
if (l_index < 0)
466-
elog(ERROR, "USING column \"%s\" not found in left table",
555+
elog(ERROR, "JOIN/USING column \"%s\" not found in left table",
467556
u_colname);
468557

469558
ndx = 0;
@@ -480,7 +569,7 @@ transformFromClauseItem(ParseState *pstate, Node *n)
480569
ndx++;
481570
}
482571
if (r_index < 0)
483-
elog(ERROR, "USING column \"%s\" not found in right table",
572+
elog(ERROR, "JOIN/USING column \"%s\" not found in right table",
484573
u_colname);
485574

486575
l_colvar = nth(l_index, l_colvars);
@@ -520,18 +609,14 @@ transformFromClauseItem(ParseState *pstate, Node *n)
520609
res_colvars = lappend(res_colvars, colvar);
521610
}
522611

523-
j->quals = transformUsingClause(pstate, l_usingvars, r_usingvars);
612+
j->quals = transformJoinUsingClause(pstate,
613+
l_usingvars,
614+
r_usingvars);
524615
}
525616
else if (j->quals)
526617
{
527618
/* User-written ON-condition; transform it */
528-
j->quals = transformExpr(pstate, j->quals, EXPR_COLUMN_FIRST);
529-
if (exprType(j->quals) != BOOLOID)
530-
{
531-
elog(ERROR, "ON clause must return type bool, not type %s",
532-
typeidTypeName(exprType(j->quals)));
533-
}
534-
/* XXX should check that ON clause refers only to joined tbls */
619+
j->quals = transformJoinOnClause(pstate, j, *containedRels);
535620
}
536621
else
537622
{

0 commit comments

Comments
 (0)