Skip to content

Commit

Permalink
[yugabyte#14278] YSQL: Make sure copy function for YbBatchedNestLoop …
Browse files Browse the repository at this point in the history
…does a deep copy for its fields

Summary:
`_copyYbBatchedNestLoop` in `copyfuncs.c` was doing a shallow copy of its `nl` field. This results in the field having invalid fields when a copy of a `YbBatchedNestLoop` plan is reused over multiple connections. This revision fixes that issue by adding the appropriate copy logic in `_copyYbBatchedNestLoop`.

This revision also fixes a couple glaring cosmetic issues in code relevant to batched nested loop joins.

Test Plan:
./yb_build.sh release --java-test 'org.yb.pgsql.TestPgMisc'

Reviewers: rskannan, amartsinchyk

Reviewed By: amartsinchyk

Differential Revision: https://phabricator.dev.yugabyte.com/D19919
  • Loading branch information
tanujnay112 committed Oct 1, 2022
1 parent df5ff36 commit caa3be6
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 4 deletions.
32 changes: 32 additions & 0 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgMisc.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,36 @@ public void testPgHintPlanExtendedQuery() throws Exception {
statement.executeUpdate("DROP TABLE test_table;");
}
}

/*
* Test to make sure that batched nested loops joins work with extended queries over
* multiple invocations.
* See issue #14278.
*/
@Test
public void testBatchedNestLoopExtendedQuery() throws Exception {
try (Statement statement = connection.createStatement()) {
statement.executeUpdate("CREATE TABLE test_table1(r1 int, r2 int," +
"PRIMARY KEY(r1 asc))");
statement.executeUpdate("CREATE TABLE test_table2(r1 int, r2 int," +
"PRIMARY KEY(r1 asc))");

String QUERY = "/*+Set(enable_hashjoin off) Set(enable_mergejoin off) " +
"Set(yb_bnl_batch_size 4) Set(enable_seqscan off) " +
"Set(enable_material off)*/ " +
"SELECT * FROM test_table1, test_table2 " +
"WHERE test_table1.r1 = test_table2.r1";
PreparedStatement stmt = connection.prepareStatement(QUERY);
// Make sure we're always preparing statements at the server
com.yugabyte.jdbc.PgStatement pgstmt = (com.yugabyte.jdbc.PgStatement) stmt;
pgstmt.setPrepareThreshold(1);

stmt.executeQuery();
stmt.executeQuery();
stmt.executeQuery();

statement.executeUpdate("DROP TABLE test_table1;");
statement.executeUpdate("DROP TABLE test_table2;");
}
}
}
5 changes: 3 additions & 2 deletions src/postgres/src/backend/nodes/copyfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -893,9 +893,10 @@ _copyYbBatchedNestLoop(const YbBatchedNestLoop *from)
YbBatchedNestLoop *newnode = makeNode(YbBatchedNestLoop);

/*
* copy NestLoop field
* copy node superclass fields
*/
COPY_SCALAR_FIELD(nl);
CopyJoinFields((const Join *) from, (Join *) newnode);
COPY_NODE_FIELD(nl.nestParams);

/*
* copy remainder of node
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/backend/optimizer/plan/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
root->inhTargetKind = INHKIND_NONE;
root->hasRecursion = hasRecursion;
root->yb_curbatchedrelids = parent_root ? parent_root->yb_curbatchedrelids
: false;
: NULL;
root->yb_cur_batch_no = -1;
if (hasRecursion)
root->wt_param_id = assign_special_exec_param(root);
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/include/executor/nodeYbBatchedNestloop.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* or implied. See the License for the specific language governing permissions and limitations
* under the License.
*
* src/postgres/src/include/executor/nodeYbBatchedNestLoop.h
* src/postgres/src/include/executor/nodeYbBatchedNestloop.h
*
*-------------------------------------------------------------------------
*/
Expand Down

0 comments on commit caa3be6

Please sign in to comment.