Skip to content

Commit 62c8a16

Browse files
authored
Merge pull request #2832 from AniruthKarthik/fix-returns-table-substitution
Fix incorrect variable substitution for column references in PL/pgSQL
2 parents e690a48 + 06c90bc commit 62c8a16

2 files changed

Lines changed: 54 additions & 14 deletions

File tree

server/plpgsql/statements.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -471,25 +471,31 @@ func substituteVariableReferences(expression string, stack *InterpreterStack) (n
471471
token := scanResult.Tokens[i]
472472
substring := expression[token.Start:token.End]
473473
// varMap lowercases everything, so we'll lowercase our substring to enable case-insensitivity
474-
if _, ok := varMap[strings.ToLower(substring)]; ok {
475-
// If there's a '.', then we'll assume this is accessing a record's field (`NEW.val1` for example)
476-
for i+2 < len(scanResult.Tokens) && scanResult.Tokens[i+1].Token == '.' {
477-
nextFieldSubstring := expression[scanResult.Tokens[i+2].Start:scanResult.Tokens[i+2].End]
478-
substring += "." + nextFieldSubstring
479-
i += 2
480-
}
481-
// Variables cannot have a '(' after their name as that would classify them as functions, so we have to
482-
// explicitly check for that. This is because variables and functions can share names, for example:
483-
// SELECT COUNT(*) INTO count FROM table_name;
484-
if i+1 >= len(scanResult.Tokens) || scanResult.Tokens[i+1].Token != '(' {
474+
isAfterDot := i > 0 && scanResult.Tokens[i-1].Token == '.'
475+
476+
if !isAfterDot {
477+
if _, ok := varMap[strings.ToLower(substring)]; ok {
478+
// If there's a '.', then we'll assume this is accessing a record's field (`NEW.val1` for example)
479+
for i+2 < len(scanResult.Tokens) && scanResult.Tokens[i+1].Token == '.' {
480+
nextFieldSubstring := expression[scanResult.Tokens[i+2].Start:scanResult.Tokens[i+2].End]
481+
substring += "." + nextFieldSubstring
482+
i += 2
483+
}
484+
// Variables cannot have a '(' after their name as that would classify them as functions, so we have to
485+
// explicitly check for that. This is because variables and functions can share names, for example:
486+
// SELECT COUNT(*) INTO count FROM table_name;
487+
if i+1 >= len(scanResult.Tokens) || scanResult.Tokens[i+1].Token != '(' {
488+
referencedVars = append(referencedVars, substring)
489+
newExpression += fmt.Sprintf("$%d ", len(referencedVars))
490+
} else {
491+
newExpression += substring + " "
492+
}
493+
} else if _, ok := triggerSpecialVariables[substring]; ok {
485494
referencedVars = append(referencedVars, substring)
486495
newExpression += fmt.Sprintf("$%d ", len(referencedVars))
487496
} else {
488497
newExpression += substring + " "
489498
}
490-
} else if _, ok := triggerSpecialVariables[substring]; ok {
491-
referencedVars = append(referencedVars, substring)
492-
newExpression += fmt.Sprintf("$%d ", len(referencedVars))
493499
} else {
494500
newExpression += substring + " "
495501
}

testing/go/create_function_plpgsql_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,40 @@ $$ LANGUAGE plpgsql;`},
704704
},
705705
},
706706
},
707+
{
708+
Name: "RETURNS TABLE with conflicting variable names",
709+
SetUpScript: []string{
710+
`CREATE TABLE customers2 (
711+
id INT PRIMARY KEY,
712+
name TEXT
713+
);`,
714+
`INSERT INTO customers2 VALUES (1, 'John'), (2, 'Jane');`,
715+
`CREATE OR REPLACE FUNCTION func_conflict(n INT) RETURNS TABLE (id INT, name TEXT)
716+
LANGUAGE plpgsql
717+
AS $$
718+
BEGIN
719+
RETURN QUERY
720+
SELECT c.id, c.name
721+
FROM customers2 c
722+
WHERE c.id = n;
723+
END;
724+
$$;`,
725+
},
726+
Assertions: []ScriptTestAssertion{
727+
{
728+
Query: "SELECT func_conflict(1);",
729+
Expected: []sql.Row{
730+
{"(1,John)"},
731+
},
732+
},
733+
{
734+
Query: "SELECT func_conflict(2);",
735+
Expected: []sql.Row{
736+
{"(2,Jane)"},
737+
},
738+
},
739+
},
740+
},
707741
{
708742
Name: "RETURNS SETOF with composite param",
709743
SetUpScript: []string{

0 commit comments

Comments
 (0)