Skip to content

Commit

Permalink
[native] Fix varchar cast for json (#24396)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
natashasehgal authored and facebook-github-bot committed Jan 24, 2025
1 parent 48f2fb1 commit 1a3f520
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,13 @@ std::optional<TypedExprPtr> convertCastToVarcharWithMaxLength(
VELOX_DCHECK(end == returnType.data() + returnType.size() - 1);

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

auto arg = args[0];
// If the argument is of JSON type, convert it to VARCHAR before applying
// substr.
if (velox::isJsonType(arg->type())) {
arg = std::make_shared<CastTypedExpr>(velox::VARCHAR(), arg, false);
}
return std::make_shared<CallTypedExpr>(
arg->type(),
std::vector<TypedExprPtr>{
Expand Down Expand Up @@ -256,8 +261,8 @@ std::optional<TypedExprPtr> tryConvertCast(
}

// When the return type is varchar with max length, truncate if only the
// argument type is varchar (or varchar with max length). Non-varchar argument
// types are not truncated.
// argument type is varchar, or varchar with max length or json. Non-varchar
// argument types are not truncated.
if (returnType.find(kVarchar) == 0 &&
args[0]->type()->kind() == TypeKind::VARCHAR &&
returnType.size() > strlen(kVarchar)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "presto_cpp/presto_protocol/core/presto_protocol_core.h"
#include "velox/core/Expressions.h"
#include "velox/type/Type.h"
#include "velox/functions/prestosql/types/JsonType.h"

using namespace facebook::presto;
using namespace facebook::velox;
Expand All @@ -30,6 +31,7 @@ class RowExpressionTest : public ::testing::Test {
}

void SetUp() override {
registerJsonType();
pool_ = memory::MemoryManager::getInstance()->addLeafPool();
converter_ =
std::make_unique<VeloxExprConverter>(pool_.get(), &typeParser_);
Expand Down Expand Up @@ -626,6 +628,29 @@ TEST_F(RowExpressionTest, castToVarchar) {
ASSERT_TRUE(returnExpr->nullOnFailure());
ASSERT_EQ(returnExpr->type()->toString(), "VARCHAR");
}
// CAST(json AS varchar(3))
{
std::shared_ptr<protocol::CallExpression> p =
json::parse(makeCastToVarchar(false, "json", "varchar(3)"));
auto expr = converter_->toVeloxExpr(p);
auto returnExpr = std::dynamic_pointer_cast<const CallTypedExpr>(expr);

ASSERT_NE(returnExpr, nullptr);
ASSERT_EQ(returnExpr->name(), "presto.default.substr");

auto returnArg1 = std::dynamic_pointer_cast<const CastTypedExpr>(
returnExpr->inputs()[0]);
auto returnArg2 = std::dynamic_pointer_cast<const ConstantTypedExpr>(
returnExpr->inputs()[1]);
auto returnArg3 = std::dynamic_pointer_cast<const ConstantTypedExpr>(
returnExpr->inputs()[2]);

ASSERT_EQ(returnArg1->type()->toString(), "VARCHAR");
ASSERT_EQ(returnArg2->type()->toString(), "BIGINT");
ASSERT_EQ(returnArg2->value().toJson(returnArg2->type()), "1");
ASSERT_EQ(returnArg3->type()->toString(), "BIGINT");
ASSERT_EQ(returnArg3->value().toJson(returnArg3->type()), "3");
}
}

TEST_F(RowExpressionTest, special) {
Expand Down

0 comments on commit 1a3f520

Please sign in to comment.