Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bind format support #4

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 101 additions & 41 deletions java/yb-cql-4x/src/test/java/org/yb/loadtest/TestTupleOperators.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.slf4j.LoggerFactory;

import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

Expand All @@ -39,6 +40,7 @@

import com.datastax.oss.driver.api.core.type.DataTypes;
import com.datastax.oss.driver.api.core.type.TupleType;
import com.datastax.oss.driver.api.core.data.TupleValue;

@RunWith(value = YBTestRunnerNonTsanOnly.class)
public class TestTupleOperators extends BaseMiniClusterTest {
Expand Down Expand Up @@ -104,54 +106,112 @@ public void testTupleBind() throws Exception {
}
assertEquals(select_row_count, 1);

// Unsupported bind format test #1
// Bind by name - one variable per tuple
selectStmt = String.format("SELECT * FROM %s WHERE (r1, r2) IN (:tup1, :tup2);", tbl);
PreparedStatement preparedSelect;
PreparedStatement preparedSelect = session.prepare(selectStmt);
TupleType tupleType = DataTypes.tupleOf(DataTypes.INT, DataTypes.TEXT);
BoundStatement boundStmt = preparedSelect.boundStatementBuilder()
.setTupleValue("tup1", tupleType.newValue(101, "r101"))
.setTupleValue("tup2", tupleType.newValue(102, "r102"))
.build();
rs = session.execute(boundStmt);

select_row_count = 0;
iter = rs.iterator();
int counter = 1;
while (iter.hasNext()) {
Row row = iter.next();
assertEquals(row.getInt(0), counter); /* h1 */
assertEquals(row.getInt(2), counter + 100); /* r1 */
assertEquals(row.getInt(4), counter + 1000); /* v1 */
select_row_count++;
counter++;
}
assertEquals(select_row_count, 2);

// Basic bind - one variable per tuple
selectStmt = String.format("SELECT * FROM %s WHERE (r1, r2) IN (?, ?);", tbl);
preparedSelect = session.prepare(selectStmt);
List<TupleValue> choices = new ArrayList<>();
choices.add(tupleType.newValue(101, "r101"));
choices.add(tupleType.newValue(102, "r102"));
rs = session.execute(preparedSelect.bind(choices.toArray(TupleValue[]::new)));
select_row_count = 0;
iter = rs.iterator();
counter = 1;
while (iter.hasNext()) {
Row row = iter.next();
assertEquals(row.getInt(0), counter); /* h1 */
assertEquals(row.getInt(2), counter + 100); /* r1 */
assertEquals(row.getInt(4), counter + 1000); /* v1 */
select_row_count++;
counter++;
}
assertEquals(select_row_count, 2);

// Invalid number of variables - one variable per tuple
List<TupleValue> invalid_choices = new ArrayList<>();
invalid_choices.add(tupleType.newValue(101, "r101"));
invalid_choices.add(tupleType.newValue(102, "r102"));
invalid_choices.add(tupleType.newValue(103, "r103"));
try {
preparedSelect = session.prepare(selectStmt);
} catch (com.datastax.oss.driver.api.core.servererrors.SyntaxError e) {
assertTrue(e.getMessage().contains("Feature Not Supported. Bind format not supported"));
rs = session.execute(preparedSelect.bind(invalid_choices.toArray(TupleValue[]::new)));
} catch (java.lang.IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Too many variables (expected 2, got 3)"));
logger.info("Expected exception", e);
}

// Use the below tests once the format is supported.
// TupleType tupleType = DataTypes.tupleOf(DataTypes.INT, DataTypes.TEXT);
// BoundStatement boundStmt = preparedSelect.boundStatementBuilder()
// .setTupleValue("tup1", tupleType.newValue(101, "r101"))
// .setTupleValue("tup2", tupleType.newValue(102, "r102"))
// .build();
// rs = session.execute(boundStmt);

// select_row_count = 0;
// iter = rs.iterator();
// int counter = 1;
// while (iter.hasNext()) {
// Row row = iter.next();
// String result = String.format("Result = %d, %s, %d, %s, %d, %s",
// row.getInt(0),
// row.getString(1),
// row.getInt(2),
// row.getString(3),
// row.getInt(4),
// row.getString(5));
// logger.info(result);

// assertEquals(row.getInt(0), counter); /* h1 */
// assertEquals(row.getInt(2), counter + 100); /* r1 */
// assertEquals(row.getInt(4), counter + 1000); /* v1 */

// select_row_count++;
// counter++;
// }
// assertEquals(select_row_count, 2);

// Unsupported bind format test #2
selectStmt = String.format("SELECT * FROM %s WHERE (r1, r2) IN ?;", tbl);
// Invalid tuple type - one variable per tuple
invalid_choices = new ArrayList<>();
TupleType invalidTupleType = DataTypes.tupleOf(DataTypes.INT, DataTypes.INT);
invalid_choices.add(invalidTupleType.newValue(101, 101));
invalid_choices.add(invalidTupleType.newValue(102, 102));
try {
preparedSelect = session.prepare(selectStmt);
} catch (com.datastax.oss.driver.api.core.servererrors.SyntaxError e) {
assertTrue(e.getMessage().contains("Feature Not Supported. Bind format not supported"));
rs = session.execute(preparedSelect.bind(invalid_choices.toArray(TupleValue[]::new)));
} catch (java.lang.IllegalArgumentException e) {
assertTrue(e.getMessage()
.contains("Invalid tuple type, expected Tuple(INT, TEXT) but got " +
"Tuple(INT, INT)"));
logger.info("Expected exception", e);
}

// Bind by name - one variable for entire list
selectStmt = String.format("SELECT * FROM %s WHERE (r1, r2) IN :choices;", tbl);
preparedSelect = session.prepare(selectStmt);
boundStmt = preparedSelect.boundStatementBuilder()
.setList("choices", choices, TupleValue.class).build();
rs = session.execute(boundStmt);

select_row_count = 0;
iter = rs.iterator();
counter = 1;
while (iter.hasNext()) {
Row row = iter.next();
assertEquals(row.getInt(0), counter); /* h1 */
assertEquals(row.getInt(2), counter + 100); /* r1 */
assertEquals(row.getInt(4), counter + 1000); /* v1 */
select_row_count++;
counter++;
}
assertEquals(select_row_count, 2);


// Basic bind - one variable for entire list
selectStmt = String.format("SELECT * FROM %s WHERE (r1, r2) IN ?;", tbl);
preparedSelect = session.prepare(selectStmt);
rs = session.execute(preparedSelect.bind(choices));

select_row_count = 0;
iter = rs.iterator();
counter = 1;
while (iter.hasNext()) {
Row row = iter.next();
assertEquals(row.getInt(0), counter); /* h1 */
assertEquals(row.getInt(2), counter + 100); /* r1 */
assertEquals(row.getInt(4), counter + 1000); /* v1 */
select_row_count++;
counter++;
}
assertEquals(select_row_count, 2);
}
}
11 changes: 11 additions & 0 deletions src/yb/common/ql_serialization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,17 @@ void SerializeValue(
}
break;
}
case TUPLE: {
const QLSeqValuePB& tuple = pb.tuple_value();
size_t num_elems = tuple.elems_size();
DCHECK_EQ(num_elems, ql_type->params().size());
int32_t start_pos = CQLStartCollection(buffer);
for (size_t i = 0; i < num_elems; i++) {
SerializeValue(ql_type->param_type(i), client, tuple.elems(static_cast<int>(i)), buffer);
}
CQLFinishCollection(start_pos, buffer);
return;
}

QL_UNSUPPORTED_TYPES_IN_SWITCH:
break;
Expand Down
13 changes: 13 additions & 0 deletions src/yb/common/ql_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,19 @@ Status QLValue::Deserialize(
break;
}

case TUPLE: {
set_tuple_value();
size_t num_elems = ql_type->params().size();
for (size_t i = 0; i < num_elems; ++i) {
const shared_ptr<QLType>& elem_type = ql_type->param_type(i);
QLValue elem;
RETURN_NOT_OK(elem.Deserialize(elem_type, client, data));
RETURN_NOT_OK(CheckForNull(elem));
*add_tuple_elem() = std::move(*elem.mutable_value());
}
return Status::OK();
}

QL_UNSUPPORTED_TYPES_IN_SWITCH:
break;

Expand Down
5 changes: 4 additions & 1 deletion src/yb/common/ql_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
// The list of unsupported datatypes to use in switch statements
#define QL_UNSUPPORTED_TYPES_IN_SWITCH \
case NULL_VALUE_TYPE: FALLTHROUGH_INTENDED; \
case TUPLE: FALLTHROUGH_INTENDED; \
case TYPEARGS: FALLTHROUGH_INTENDED; \
case UNKNOWN_DATA

Expand Down Expand Up @@ -428,6 +427,9 @@ class QLValue {
QLValuePB* add_frozen_elem() {
return pb_.mutable_frozen_value()->add_elems();
}
QLValuePB* add_tuple_elem() {
return pb_.mutable_tuple_value()->add_elems();
}

// For collections, the call to `mutable_foo` takes care of setting the correct type to `foo`
// internally and allocating the message if needed
Expand All @@ -444,6 +446,7 @@ class QLValue {
void set_frozen_value() {
pb_.mutable_frozen_value();
}
void set_tuple_value() { pb_.mutable_tuple_value(); }

//----------------------------------- assignment methods ----------------------------------
QLValue& operator=(const QLValuePB& other) {
Expand Down
1 change: 1 addition & 0 deletions src/yb/common/types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class TypeInfoResolver {
AddMapping<TIMEUUID>();
AddMapping<USER_DEFINED_TYPE>();
AddMapping<FROZEN>();
AddMapping<TUPLE>();
}

template<DataType type> void AddMapping() {
Expand Down
9 changes: 9 additions & 0 deletions src/yb/common/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,15 @@ struct DataTypeTraits<FROZEN> : public DerivedTypeTraits<BINARY>{
// of Kudu Slice [ENG-1235]
};

template <>
struct DataTypeTraits<TUPLE> : public DerivedTypeTraits<BINARY> {
static const char *name() { return "tuple"; }

// using the default implementation inherited from BINARY for AppendDebugStringForValue
// TODO much of this codepath should be retired and we should systematically use QLValue instead
// of Kudu Slice [ENG-1235]
};

template<>
struct DataTypeTraits<DECIMAL> : public DerivedTypeTraits<BINARY>{
static const char* name() {
Expand Down
26 changes: 7 additions & 19 deletions src/yb/yql/cql/ql/ptree/pt_dml.cc
Original file line number Diff line number Diff line change
Expand Up @@ -551,32 +551,20 @@ Status WhereExprState::AnalyzeMultiColumnOp(
ErrorCode::CQL_STATEMENT_INVALID);
}

if (value->expr_op() != ExprOperator::kCollection) {
if (value->expr_op() == ExprOperator::kBindVar) {
return sem_context->Error(
value, "Bind format not supported", ErrorCode::FEATURE_NOT_SUPPORTED);
}
if (value->expr_op() != ExprOperator::kCollection && value->expr_op() != ExprOperator::kBindVar) {
return sem_context->Error(
value, "Invalid operand for IN clause", ErrorCode::CQL_STATEMENT_INVALID);
}

const auto options = static_cast<const PTCollectionExpr*>(value.get());
if (value->expr_op() == ExprOperator::kCollection) {
const auto options = static_cast<const PTCollectionExpr*>(value.get());

for (const auto& option : options->values()) {
if (option->expr_op() != ExprOperator::kCollection) {
if (option->expr_op() == ExprOperator::kBindVar) {
for (const auto& option : options->values()) {
if (option->expr_op() != ExprOperator::kCollection &&
option->expr_op() != ExprOperator::kBindVar) {
return sem_context->Error(
option, "Bind format not supported", ErrorCode::FEATURE_NOT_SUPPORTED);
option, "Invalid operand for IN clause", ErrorCode::CQL_STATEMENT_INVALID);
}
return sem_context->Error(
option, "Invalid operand for IN clause", ErrorCode::CQL_STATEMENT_INVALID);
}

const auto option_as_collection = static_cast<const PTCollectionExpr*>(option.get());
if (option_as_collection->values().size() != col_descs.size()) {
return sem_context->Error(
option, "Columns and value size mismatch for IN clause",
ErrorCode::CQL_STATEMENT_INVALID);
}
}

Expand Down
Loading