Skip to content

Commit 997643c

Browse files
natashasehgalfacebook-github-bot
authored andcommitted
[native] Fix varchar cast for json (prestodb#24396)
Summary: https://www.internalfb.com/tasks/?t=211442303 There was an error in running query on Prestissimo not Presto - "Scalar function presto.default.substr not registered with arguments: (JSON, BIGINT, BIGINT)". This is not due to missing function, as the function signature does not exist in Presto. It occurs when attempting to cast JSON as varchar of capped length. Related Diff: https://www.internalfb.com/diff/D59531026 Note: Exception is still raised for try_cast() behavior. Alignment is out of scope for this PR Differential Revision: D68353517
1 parent 48f2fb1 commit 997643c

File tree

2 files changed

+33
-3
lines changed

2 files changed

+33
-3
lines changed

presto-native-execution/presto_cpp/main/types/PrestoToVeloxExpr.cpp

+8-3
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,13 @@ std::optional<TypedExprPtr> convertCastToVarcharWithMaxLength(
181181
VELOX_DCHECK(end == returnType.data() + returnType.size() - 1);
182182

183183
VELOX_DCHECK_EQ(args.size(), 1);
184-
const auto arg = args[0];
185184

185+
auto arg = args[0];
186+
// If the argument is of JSON type, convert it to VARCHAR before applying
187+
// substr.
188+
if (velox::isJsonType(arg->type())) {
189+
arg = std::make_shared<CastTypedExpr>(velox::VARCHAR(), arg, false);
190+
}
186191
return std::make_shared<CallTypedExpr>(
187192
arg->type(),
188193
std::vector<TypedExprPtr>{
@@ -256,8 +261,8 @@ std::optional<TypedExprPtr> tryConvertCast(
256261
}
257262

258263
// When the return type is varchar with max length, truncate if only the
259-
// argument type is varchar (or varchar with max length). Non-varchar argument
260-
// types are not truncated.
264+
// argument type is varchar, or varchar with max length or json. Non-varchar
265+
// argument types are not truncated.
261266
if (returnType.find(kVarchar) == 0 &&
262267
args[0]->type()->kind() == TypeKind::VARCHAR &&
263268
returnType.size() > strlen(kVarchar)) {

presto-native-execution/presto_cpp/main/types/tests/RowExpressionTest.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "presto_cpp/presto_protocol/core/presto_protocol_core.h"
1919
#include "velox/core/Expressions.h"
2020
#include "velox/type/Type.h"
21+
#include "velox/functions/prestosql/types/JsonType.h"
2122

2223
using namespace facebook::presto;
2324
using namespace facebook::velox;
@@ -30,6 +31,7 @@ class RowExpressionTest : public ::testing::Test {
3031
}
3132

3233
void SetUp() override {
34+
registerJsonType();
3335
pool_ = memory::MemoryManager::getInstance()->addLeafPool();
3436
converter_ =
3537
std::make_unique<VeloxExprConverter>(pool_.get(), &typeParser_);
@@ -626,6 +628,29 @@ TEST_F(RowExpressionTest, castToVarchar) {
626628
ASSERT_TRUE(returnExpr->nullOnFailure());
627629
ASSERT_EQ(returnExpr->type()->toString(), "VARCHAR");
628630
}
631+
// CAST(json AS varchar(3))
632+
{
633+
std::shared_ptr<protocol::CallExpression> p =
634+
json::parse(makeCastToVarchar(false, "json", "varchar(3)"));
635+
auto expr = converter_->toVeloxExpr(p);
636+
auto returnExpr = std::dynamic_pointer_cast<const CallTypedExpr>(expr);
637+
638+
ASSERT_NE(returnExpr, nullptr);
639+
ASSERT_EQ(returnExpr->name(), "presto.default.substr");
640+
641+
auto returnArg1 = std::dynamic_pointer_cast<const CastTypedExpr>(
642+
returnExpr->inputs()[0]);
643+
auto returnArg2 = std::dynamic_pointer_cast<const ConstantTypedExpr>(
644+
returnExpr->inputs()[1]);
645+
auto returnArg3 = std::dynamic_pointer_cast<const ConstantTypedExpr>(
646+
returnExpr->inputs()[2]);
647+
648+
ASSERT_EQ(returnArg1->type()->toString(), "VARCHAR");
649+
ASSERT_EQ(returnArg2->type()->toString(), "BIGINT");
650+
ASSERT_EQ(returnArg2->value().toJson(returnArg2->type()), "1");
651+
ASSERT_EQ(returnArg3->type()->toString(), "BIGINT");
652+
ASSERT_EQ(returnArg3->value().toJson(returnArg3->type()), "3");
653+
}
629654
}
630655

631656
TEST_F(RowExpressionTest, special) {

0 commit comments

Comments
 (0)