fix(4paradigm#1761): explain require next <cr> and space ending probl…

…em (4paradigm#1820)

close 4paradigm#1761, two problem in cli:

1. explain result missing a <CR>
2. fail to precess ending space

hybridse/src/vm/sql_compiler.cc

@@ -96,7 +96,7 @@ bool SqlCompiler::Compile(SqlContext& ctx, Status& status) { // NOLINT
 
  if (dump_plan_) {
  std::stringstream physical_plan_ss;
- ctx.physical_plan->Print(physical_plan_ss, "\t");
+ ctx.physical_plan->Print(physical_plan_ss, "");
  ctx.physical_plan_str = physical_plan_ss.str();
  }
  ok = codec::SchemaCodec::Encode(ctx.schema, &ctx.encoded_schema);

src/cmd/sql_cmd.h

@@ -63,6 +63,27 @@ const std::string VERSION = std::to_string(OPENMLDB_VERSION_MAJOR) + "." + // N
 ::openmldb::sdk::DBSDK* cs = nullptr;
 ::openmldb::sdk::SQLClusterRouter* sr = nullptr;
 
+// strip any whitespace characters begining of the last unfinished statement from `input` (string after last semicolon)
+// final SQL strings are appended into `output`
+//
+// this help handle SQL strings that has space trailing but do not expected to be a statement after semicolon
+void StripStartingSpaceOfLastStmt(absl::string_view input, std::string* output) {
+ auto last_semicolon_pos = input.find_last_of(';');
+ if (last_semicolon_pos != std::string::npos && input.back() != ';') {
+ absl::string_view last_stmt = input;
+ last_stmt.remove_prefix(last_semicolon_pos + 1);
+ while (!last_stmt.empty() && std::isspace(static_cast<unsigned char>(last_stmt.front()))) {
+ last_stmt.remove_prefix(1);
+ }
+ output->append(input.begin(), input.begin() + last_semicolon_pos + 1);
+ if (!last_stmt.empty()) {
+ output->append(last_stmt);
+ }
+ } else {
+ output->append(input);
+ }
+}
+
 void HandleSQL(const std::string& sql) {
  hybridse::sdk::Status status;
  auto result_set = sr->ExecuteSQL(sql, &status);
@@ -154,21 +175,8 @@ void Shell() {
  }
  // todo: should support multiple sql.
  // trim space after last semicolon in sql
- auto last_semicolon_pos = buffer.find_last_of(';');
- if (last_semicolon_pos != std::string::npos && buffer.back() != ';') {
- absl::string_view input = buffer;
- input.remove_prefix(last_semicolon_pos);
- std::string prefix(" ");
- while (true) {
- if (!absl::ConsumePrefix(&input, prefix)) {
- break;
- }
- }
- sql.append(buffer.begin(), buffer.begin() + last_semicolon_pos + 1);
- sql.append(input.begin(), input.end());
- } else {
- sql.append(buffer);
- }
+ StripStartingSpaceOfLastStmt(buffer, &sql);
+
  if (sql.length() == 4 || sql.length() == 5) {
  if (absl::EqualsIgnoreCase(sql, "quit;") || absl::EqualsIgnoreCase(sql, "exit;") ||
  absl::EqualsIgnoreCase(sql, "quit") || absl::EqualsIgnoreCase(sql, "exit")) {

src/cmd/sql_cmd_test.cc

@@ -1969,6 +1969,27 @@ TEST_P(DBSDKTest, DeployStatsOnlyCollectDeployProcedure) {
  }
 }
 
+class StripSpaceTest : public ::testing::TestWithParam<std::pair<std::string_view, std::string_view>> {};
+
+std::vector<std::pair<std::string_view, std::string_view>> strip_cases = {
+ {"show components;", "show components;"},
+ {"show components; ", "show components;"},
+ {"show components;\t", "show components;"},
+ {"show components; \t", "show components;"},
+ {"show components; \v\t\r\n\f", "show components;"},
+ {"show components; show", "show components;show"},
+};
+
+INSTANTIATE_TEST_SUITE_P(Strip, StripSpaceTest, ::testing::ValuesIn(strip_cases));
+
+TEST_P(StripSpaceTest, Correctness) {
+ auto& cs = GetParam();
+
+ std::string output;
+ StripStartingSpaceOfLastStmt(cs.first, &output);
+ EXPECT_EQ(cs.second, output);
+}
+
 /* TODO: Only run test in standalone mode
 TEST_P(DBSDKTest, load_data) {
  auto cli = GetParam();

src/sdk/sql_cluster_router.cc

@@ -1291,10 +1291,10 @@ std::shared_ptr<ExplainInfo> SQLClusterRouter::Explain(const std::string& db, co
  }
  ::hybridse::sdk::SchemaImpl input_schema(explain_output.input_schema);
  ::hybridse::sdk::SchemaImpl output_schema(explain_output.output_schema);
- std::shared_ptr<ExplainInfoImpl> impl(
-  new ExplainInfoImpl(input_schema, output_schema, explain_output.logical_plan, explain_output.physical_plan,
- explain_output.ir, explain_output.request_db_name, explain_output.request_name));
- return impl;
+
+ return std::make_shared<ExplainInfoImpl>(input_schema, output_schema, explain_output.logical_plan,
+  explain_output.physical_plan, explain_output.ir,
+  explain_output.request_db_name, explain_output.request_name);
 }
 
 std::shared_ptr<hybridse::sdk::ResultSet> SQLClusterRouter::CallProcedure(const std::string& db,
@@ -1800,7 +1800,7 @@ base::Status SQLClusterRouter::HandleSQLCreateTable(hybridse::node::CreatePlanNo
  std::vector<std::shared_ptr<::openmldb::catalog::TabletAccessor>> all_tablet;
  all_tablet = cluster_sdk_->GetAllTablet();
  // set dafault value
- uint32_t default_replica_num = std::min((uint32_t)all_tablet.size(), FLAGS_replica_num);
+ uint32_t default_replica_num = std::min(static_cast<uint32_t>(all_tablet.size()), FLAGS_replica_num);
 
  hybridse::base::Status sql_status;
  bool is_cluster_mode = cluster_sdk_->IsClusterMode();
@@ -2260,7 +2260,7 @@ std::shared_ptr<hybridse::sdk::ResultSet> SQLClusterRouter::ExecuteSQL(const std
  return {};
  }
  *status = {};
- std::vector<std::string> value = {info->GetPhysicalPlan()};
+ std::vector<std::string> value = {info->GetPhysicalPlan() + "\n"};
  return ResultSetSQL::MakeResultSet({FORMAT_STRING_KEY}, {value}, status);
  }
  case hybridse::node::kPlanTypeCreate: {
@@ -3055,7 +3055,8 @@ hybridse::sdk::Status SQLClusterRouter::GetNewIndex(
  // update ttl
  auto ns_ptr = cluster_sdk_->GetNsClient();
  std::string err;
- bool ok = ns_ptr->UpdateTTL(table_name, type, new_abs_ttl, new_lat_ttl, column_key.index_name(), err);
+ bool ok =
+ ns_ptr->UpdateTTL(table_name, type, new_abs_ttl, new_lat_ttl, column_key.index_name(), err);
  if (!ok) {
  return {::hybridse::common::StatusCode::kCmdError, "update ttl failed"};
  }