Skip to content

Commit

Permalink
[#18082] YSQL: Stop using ForeignScan for YB relations
Browse files Browse the repository at this point in the history
Summary:
Since implementation of aggregate pushdown for scan types other than
ForeignScan, the ForeignScan and the YbSeqScan are functionally
equivalent, so we switch to using the YbSeqScan, as it is more unified
with IndexScan and IndexOnlyScan, which are also used to scan YB
relation. Unification should make future developments easier.

For consistency with previous design decision explain now shows "Seq Scan"
if the plan node is a YbSeqScan node, hiding implementation details from the
end user. Though YbSeqScan shows "Filter" and "Remote Filter" in different
order, if both are present, so there are changes in the expected EXPLAIN output.
The order of YbSeqScan filters consistent with  the order of index scan and
matches their evaluation order (remote filter goes first).

There is no known way to force ForeignScan-based scan on Yugabyte tables.
To help catch such cases, if any, the ForeignScan plan node on Yugabyte tables
now shows "YB Foreign Scan" in the EXPLAIN output.
Jira: DB-7124

Test Plan:
Make sure existing regressions tests work.
Modifications to expected output are needed due to differencies in
EXPLAIN output.

Reviewers: jason, telgersma

Reviewed By: jason, telgersma

Subscribers: ssong, yql

Differential Revision: https://phorge.dev.yugabyte.com/D27297
  • Loading branch information
andrei-mart committed Sep 5, 2023
1 parent 67aba99 commit 465ee2c
Show file tree
Hide file tree
Showing 27 changed files with 336 additions and 391 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public class ExplainAnalyzeUtils {
public static final String NODE_SEQ_SCAN = "Seq Scan";
public static final String NODE_SORT = "Sort";
public static final String NODE_VALUES_SCAN = "Values Scan";

public static final String NODE_YB_BATCHED_NESTED_LOOP = "YB Batched Nested Loop";
public static final String NODE_YB_SEQ_SCAN = "YB Seq Scan";

public static final String OPERATION_INSERT = "Insert";
public static final String OPERATION_UPDATE = "Update";
Expand Down
17 changes: 3 additions & 14 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgAutoExplain.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@
package org.yb.pgsql;

import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_ONLY_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_VALUES_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_NESTED_LOOP;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_MODIFY_TABLE;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_FUNCTION_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_RESULT;
import static org.yb.AssertionWrappers.assertEquals;
import static org.yb.AssertionWrappers.assertTrue;

Expand All @@ -31,7 +23,6 @@
import java.util.List;
import java.util.ArrayList;
import java.util.Collections;
import java.lang.Character;
import java.io.StringWriter;
import java.io.PrintWriter;

Expand All @@ -43,14 +34,12 @@
import org.yb.util.json.Checker;
import org.yb.util.json.Checkers;
import org.yb.util.json.JsonUtil;
import org.yb.util.json.ObjectChecker;
import org.yb.pgsql.ExplainAnalyzeUtils.PlanCheckerBuilder;
import org.yb.pgsql.ExplainAnalyzeUtils.TopLevelCheckerBuilder;
import org.yb.minicluster.LogErrorListener;
import org.yb.minicluster.MiniYBDaemon;
import org.yb.util.BuildTypeUtil;

import org.yb.util.json.ValueChecker;
import org.yb.YBTestRunner;

import com.google.gson.JsonElement;
Expand Down Expand Up @@ -166,7 +155,7 @@ public void testDistOn() throws Exception {
TABLE_NAME, TABLE_NAME),
makeTopLevelBuilder()
.plan(makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
.relationName(TABLE_NAME)
.alias(TABLE_NAME)
.storageTableReadRequests(Checkers.greaterOrEqual(0))
Expand All @@ -187,7 +176,7 @@ public void testDistOff() throws Exception {
TABLE_NAME, TABLE_NAME),
makeTopLevelBuilder()
.plan(makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
.relationName(TABLE_NAME)
.alias(TABLE_NAME)
.actualStartupTime(Checkers.greaterOrEqual(0.0))
Expand All @@ -211,7 +200,7 @@ public void testAnalyzeOff() throws Exception {
TABLE_NAME, TABLE_NAME),
makeTopLevelBuilder()
.plan(makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
// Startup cost and total cost because auto_explain
.startupCost(Checkers.greaterOrEqual(0.0))
.totalCost(Checkers.greaterOrEqual(0.0))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_MERGE_JOIN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SORT;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_SEQ_SCAN;
import org.yb.pgsql.ExplainAnalyzeUtils.PlanCheckerBuilder;
import org.yb.pgsql.ExplainAnalyzeUtils.TopLevelCheckerBuilder;

Expand Down Expand Up @@ -167,7 +166,7 @@ public void testMergeJoinWithIndex() throws Exception {
.planRows(Checkers.greater(1))
.plans(
makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
.relationName(TABLE_NAME)
.alias("t2")
.planRows(Checkers.equal(TABLE_ROWS))
Expand All @@ -185,7 +184,7 @@ public void testMergeJoinWithIndex() throws Exception {
.nodeType(NODE_SORT)
.planRows(Checkers.greater(1))
.plans(makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
.relationName(TABLE_NAME)
.alias("t1")
.planRows(Checkers.greater(1))
Expand All @@ -199,7 +198,7 @@ public void testMergeJoinWithIndex() throws Exception {
.nodeType(NODE_SORT)
.planRows(Checkers.equal(TABLE_ROWS))
.plans(makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
.nodeType(NODE_SEQ_SCAN)
.relationName(TABLE_NAME)
.alias("t3")
.planRows(Checkers.equal(TABLE_ROWS))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,16 @@

package org.yb.pgsql;

import static org.yb.AssertionWrappers.assertEquals;
import static org.yb.AssertionWrappers.assertGreaterThan;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.Streams;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.yb.util.YBTestRunnerNonTsanOnly;

import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import java.util.Arrays;

import com.google.common.collect.Range;
Expand All @@ -45,7 +33,6 @@
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_ONLY_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_SEQ_SCAN;

@RunWith(YBTestRunnerNonTsanOnly.class)
public class TestPgColumnReadEfficiency extends BasePgSQLTest {
Expand Down Expand Up @@ -127,7 +114,7 @@ public void testScanRowSize() throws Exception {
ssEstimate = estimatedRpcsPerCol * 2;
}

testSpecificScanRowSize(stmt, NODE_YB_SEQ_SCAN, select, where, ssEstimate, N_ROWS);
testSpecificScanRowSize(stmt, NODE_SEQ_SCAN, select, where, ssEstimate, N_ROWS);
testSpecificScanRowSize(stmt, NODE_INDEX_ONLY_SCAN, select, where,
iosEstimate, N_ROWS);
if (nConditionCols > 0)
Expand Down Expand Up @@ -166,9 +153,9 @@ public void testScanRowSizeHashCode() throws Exception {
if (nSelectedCols == 0) {
ssEstimateWithoutCond = estimatedRpcsPerCol * 6;
}
testSpecificScanRowSize(stmt, NODE_YB_SEQ_SCAN, select, hashcodeCond,
testSpecificScanRowSize(stmt, NODE_SEQ_SCAN, select, hashcodeCond,
ssEstimateWithCond, N_ROWS_MATCHING_COND);
testSpecificScanRowSize(stmt, NODE_YB_SEQ_SCAN, select, "",
testSpecificScanRowSize(stmt, NODE_SEQ_SCAN, select, "",
ssEstimateWithoutCond, N_ROWS);
}

Expand All @@ -190,7 +177,7 @@ public void testScanRowSizeHashCode() throws Exception {
}
testSpecificScanRowSize(stmt, NODE_INDEX_ONLY_SCAN, select, hashcodeCond,
iosEstimateWithCond, N_ROWS_MATCHING_COND);
testSpecificScanRowSize(stmt, NODE_YB_SEQ_SCAN, select, "",
testSpecificScanRowSize(stmt, NODE_SEQ_SCAN, select, "",
iosEstimateWithoutCond, N_ROWS);
}
}
Expand All @@ -200,7 +187,7 @@ private void testSpecificScanRowSize(Statement stmt, String scanType, String sel
String conditions, long expected, int expectedRows) throws Exception {
String hint;
switch(scanType) {
case NODE_YB_SEQ_SCAN:
case NODE_SEQ_SCAN:
hint = "SeqScan(t)";
break;
case NODE_INDEX_ONLY_SCAN:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_INDEX_ONLY_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_SEQ_SCAN;

import java.sql.Statement;

Expand Down Expand Up @@ -140,8 +139,8 @@ public void testYbSeqScan() throws Exception {
final String queryWithExpr = String.format("/*+ SeqScan(foo) */ SELECT * FROM %s "
+ "WHERE v5 < 64", MAIN_TABLE);

PlanCheckerBuilder YB_SEQ_SCAN_PLAN = makePlanBuilder()
.nodeType(NODE_YB_SEQ_SCAN)
PlanCheckerBuilder SEQ_SCAN_PLAN = makePlanBuilder()
.nodeType(NODE_SEQ_SCAN)
.relationName(MAIN_TABLE)
.alias(MAIN_TABLE)
.storageTableReadExecutionTime(Checkers.greater(0.0));
Expand All @@ -150,7 +149,7 @@ public void testYbSeqScan() throws Exception {
Checker checker = SCAN_TOP_LEVEL_CHECKER
.storageReadRequests(Checkers.equal(NUM_PAGES + 1))
.storageReadExecutionTime(Checkers.greater(0.0))
.plan(YB_SEQ_SCAN_PLAN
.plan(SEQ_SCAN_PLAN
.storageTableReadRequests(Checkers.equal(NUM_PAGES + 1))
.build())
.build();
Expand All @@ -161,7 +160,7 @@ public void testYbSeqScan() throws Exception {
Checker pushdown_checker = SCAN_TOP_LEVEL_CHECKER
.storageReadRequests(Checkers.equal(3))
.storageReadExecutionTime(Checkers.greater(0.0))
.plan(YB_SEQ_SCAN_PLAN
.plan(SEQ_SCAN_PLAN
.storageTableReadRequests(Checkers.equal(3))
.build())
.build();
Expand All @@ -172,7 +171,7 @@ public void testYbSeqScan() throws Exception {
Checker no_pushdown_checker = SCAN_TOP_LEVEL_CHECKER
.storageReadRequests(Checkers.equal(NUM_PAGES + 1))
.storageReadExecutionTime(Checkers.greater(0.0))
.plan(YB_SEQ_SCAN_PLAN
.plan(SEQ_SCAN_PLAN
.storageTableReadRequests(Checkers.equal(NUM_PAGES + 1))
.build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import static org.yb.AssertionWrappers.assertEquals;
Expand All @@ -34,7 +32,6 @@
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_LIMIT;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_SEQ_SCAN;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_BATCHED_NESTED_LOOP;
import static org.yb.pgsql.ExplainAnalyzeUtils.NODE_YB_SEQ_SCAN;

import org.yb.util.json.Checker;
import org.yb.util.json.Checkers;
Expand Down Expand Up @@ -87,13 +84,9 @@ public void testSimplePrefetch() throws Exception {
tableName, tableRowCount - 1));

String seqScanQuery = String.format("SELECT * FROM %s", tableName);
String ybSeqScanQuery = String.format("/*+ SeqScan(%s) */ SELECT * FROM %s",
tableName, tableName);

checkReadRequests(statement, seqScanQuery, NODE_SEQ_SCAN,
Checkers.equal(51), tableRowCount);
checkReadRequests(statement, ybSeqScanQuery, NODE_YB_SEQ_SCAN,
Checkers.equal(51), tableRowCount);
}
}

Expand Down Expand Up @@ -121,16 +114,12 @@ public void testSizeBasedFetch() throws Exception {
statement.execute(String.format("create index on %s (v asc)", tableName));

String seqScanQuery = String.format("SELECT * FROM %s", tableName);
String ybSeqScanQuery = String.format("/*+ SeqScan(%s) */ SELECT * FROM %s",
tableName, tableName);
String indexScanQuery = String.format("SELECT * FROM %s WHERE k > 0", tableName);
String indexOnlyScanQuery = String.format("SELECT v FROM %s WHERE v > ''", tableName);

setRowAndSizeLimit(statement, 1024, 0);
checkReadRequests(statement, seqScanQuery, NODE_SEQ_SCAN,
Checkers.equal(98), tableRowCount);
checkReadRequests(statement, ybSeqScanQuery, NODE_YB_SEQ_SCAN,
Checkers.equal(98), tableRowCount);
checkReadRequests(statement, indexScanQuery, NODE_INDEX_SCAN,
Checkers.equal(98), tableRowCount);
checkReadRequests(statement, indexOnlyScanQuery, NODE_INDEX_ONLY_SCAN,
Expand All @@ -139,8 +128,6 @@ public void testSizeBasedFetch() throws Exception {
setRowAndSizeLimit(statement, 0, 0);
checkReadRequests(statement, seqScanQuery, NODE_SEQ_SCAN,
Checkers.equal(1), tableRowCount);
checkReadRequests(statement, ybSeqScanQuery, NODE_YB_SEQ_SCAN,
Checkers.equal(1), tableRowCount);
checkReadRequests(statement, indexScanQuery, NODE_INDEX_SCAN,
Checkers.equal(1), tableRowCount);
checkReadRequests(statement, indexOnlyScanQuery, NODE_INDEX_ONLY_SCAN,
Expand All @@ -150,8 +137,6 @@ public void testSizeBasedFetch() throws Exception {
setRowAndSizeLimit(statement, 0, 512);
checkReadRequests(statement, seqScanQuery, NODE_SEQ_SCAN,
Checkers.equal(25), tableRowCount);
checkReadRequests(statement, ybSeqScanQuery, NODE_YB_SEQ_SCAN,
Checkers.equal(25), tableRowCount);
checkReadRequests(statement, indexScanQuery, NODE_INDEX_SCAN,
Checkers.equal(25), tableRowCount);
checkReadRequests(statement, indexOnlyScanQuery, NODE_INDEX_ONLY_SCAN,
Expand All @@ -160,8 +145,6 @@ public void testSizeBasedFetch() throws Exception {
setRowAndSizeLimit(statement, 0, 1024);
checkReadRequests(statement, seqScanQuery, NODE_SEQ_SCAN,
Checkers.equal(13), tableRowCount);
checkReadRequests(statement, ybSeqScanQuery, NODE_YB_SEQ_SCAN,
Checkers.equal(13), tableRowCount);
checkReadRequests(statement, indexScanQuery, NODE_INDEX_SCAN,
Checkers.equal(13), tableRowCount);
checkReadRequests(statement, indexOnlyScanQuery, NODE_INDEX_ONLY_SCAN,
Expand Down
6 changes: 3 additions & 3 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbBackup.java
Original file line number Diff line number Diff line change
Expand Up @@ -1715,7 +1715,7 @@ public void testPgHintPlan() throws Exception {
" VALUES ('EXPLAIN (COSTS false) SELECT * FROM tbl1 WHERE tbl1.k = ?'," +
" '', 'SeqScan(tbl1)')");
assertQuery(stmt, "EXPLAIN (COSTS false) SELECT * FROM tbl1 WHERE tbl1.k = 1",
new Row("YB Seq Scan on tbl1"),
new Row("Seq Scan on tbl1"),
new Row(" Remote Filter: (k = 1)"));
stmt.execute("SET pg_hint_plan.enable_hint_table = off");
assertQuery(stmt, "EXPLAIN (COSTS false) SELECT * FROM tbl1 WHERE tbl1.k = 1",
Expand All @@ -1738,10 +1738,10 @@ public void testPgHintPlan() throws Exception {
" '', 'SeqScan(tbl2)')");
stmt.execute("SET pg_hint_plan.enable_hint_table = on");
assertQuery(stmt, "EXPLAIN (COSTS false) SELECT * FROM tbl1 WHERE tbl1.k = 1",
new Row("YB Seq Scan on tbl1"),
new Row("Seq Scan on tbl1"),
new Row(" Remote Filter: (k = 1)"));
assertQuery(stmt, "EXPLAIN (COSTS false) SELECT * FROM tbl2 WHERE tbl2.k = 3",
new Row("YB Seq Scan on tbl2"),
new Row("Seq Scan on tbl2"),
new Row(" Remote Filter: (k = 3)"));
stmt.execute("SET pg_hint_plan.enable_hint_table = off");
assertQuery(stmt, "EXPLAIN (COSTS false) SELECT * FROM tbl1 WHERE tbl1.k = 1",
Expand Down
4 changes: 2 additions & 2 deletions src/postgres/src/backend/commands/explain.c
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
pname = sname = "Seq Scan";
break;
case T_YbSeqScan:
pname = sname = "YB Seq Scan";
pname = sname = "Seq Scan";
break;
case T_SampleScan:
pname = sname = "Sample Scan";
Expand Down Expand Up @@ -1365,7 +1365,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
case CMD_SELECT:
/* Don't need to expose implementation details */
if (IsYBRelation(((ScanState*) planstate)->ss_currentRelation))
sname = pname = "Seq Scan";
sname = pname = "YB Foreign Scan";
else
pname = "Foreign Scan";
operation = "Select";
Expand Down
14 changes: 1 addition & 13 deletions src/postgres/src/backend/optimizer/path/allpaths.c
Original file line number Diff line number Diff line change
Expand Up @@ -525,19 +525,7 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
else
{
/* Plain relation */
if (IsYBRelationById(rte->relid))
{
/*
* Using a foreign scan which will use the YB FDW by
* default.
*/
set_foreign_pathlist(root, rel, rte);
}
else
{
/* Use regular scan for initdb tables. */
set_plain_rel_pathlist(root, rel, rte);
}
set_plain_rel_pathlist(root, rel, rte);
}
break;
case RTE_SUBQUERY:
Expand Down

0 comments on commit 465ee2c

Please sign in to comment.