Skip to content

Commit

Permalink
[#13508] DocDB: Handle not ordered columns while generating packed row
Browse files Browse the repository at this point in the history
Summary:
Row packer expected that columns are passed in the same order as they listed in the schema.
But in case of covering indexes, insert request could contain columns in a different order.
As result packing is failed.

Fixed by reordering columns when they are not sorted.

Test Plan: PgPackedRowTest.CoveringIndex

Reviewers: dmitry, bogdan, mbautin

Reviewed By: mbautin

Subscribers: kannan, rthallam, qhu, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D18542
  • Loading branch information
spolitov committed Aug 8, 2022
1 parent b3a6894 commit 3b0c447
Show file tree
Hide file tree
Showing 7 changed files with 263 additions and 122 deletions.
4 changes: 2 additions & 2 deletions src/yb/common/ql_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void LWExprResultWriter::SetNull() {
yb::SetNull(&NewValue());
}

bfql::TSOpcode QLExprExecutor::GetTSWriteInstruction(const QLExpressionPB& ql_expr) const {
bfql::TSOpcode GetTSWriteInstruction(const QLExpressionPB& ql_expr) {
// "kSubDocInsert" instructs the tablet server to insert a new value or replace an existing value.
if (ql_expr.has_tscall()) {
return static_cast<bfql::TSOpcode>(ql_expr.tscall().opcode());
Expand Down Expand Up @@ -437,7 +437,7 @@ Status QLExprExecutor::EvalCondition(const QLConditionPB& condition,

//--------------------------------------------------------------------------------------------------

bfpg::TSOpcode QLExprExecutor::GetTSWriteInstruction(const PgsqlExpressionPB& ql_expr) const {
bfpg::TSOpcode GetTSWriteInstruction(const PgsqlExpressionPB& ql_expr) {
// "kSubDocInsert" instructs the tablet server to insert a new value or replace an existing value.
if (ql_expr.has_tscall()) {
return static_cast<bfpg::TSOpcode>(ql_expr.tscall().opcode());
Expand Down
10 changes: 4 additions & 6 deletions src/yb/common/ql_expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,6 @@ class QLExprExecutor {
//------------------------------------------------------------------------------------------------
// CQL Support.

// Get TServer opcode.
yb::bfql::TSOpcode GetTSWriteInstruction(const QLExpressionPB& ql_expr) const;

// Evaluate the given QLExpressionPB.
Status EvalExpr(const QLExpressionPB& ql_expr,
const QLTableRow& table_row,
Expand Down Expand Up @@ -310,9 +307,6 @@ class QLExprExecutor {
//------------------------------------------------------------------------------------------------
// PGSQL Support.

// Get TServer opcode.
bfpg::TSOpcode GetTSWriteInstruction(const PgsqlExpressionPB& ql_expr) const;

// Evaluate the given QLExpressionPB.
Status EvalExpr(const PgsqlExpressionPB& ql_expr,
const QLTableRow* table_row,
Expand Down Expand Up @@ -407,6 +401,10 @@ Status EvalOperands(
return EvalOperandsHelper(executor, operands.begin(), table_row, std::forward<Args>(args)...);
}

// Get TServer opcode.
yb::bfql::TSOpcode GetTSWriteInstruction(const QLExpressionPB& ql_expr);
bfpg::TSOpcode GetTSWriteInstruction(const PgsqlExpressionPB& ql_expr);

} // namespace yb

#endif // YB_COMMON_QL_EXPR_H_
2 changes: 1 addition & 1 deletion src/yb/docdb/doc_pg_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class DocPgExprExecutor::Private {
YbgSetCurrentMemoryContext(row_ctx_, &old);
for (const DocPgEvalExprData& target : targets_) {
// Container for the DocDB result
QLExprResult &result = results->at(i++);
QLExprResult &result = (*results)[i++];
// Containers for Postgres result
uint64_t datum;
bool is_null;
Expand Down
Loading

0 comments on commit 3b0c447

Please sign in to comment.