Loose-ends for PostgreSQL support

Summary:
This diff fixed issues that @karthik found.
- Added test suite. The tool cannot handle comparison for error messages yet. So if the test has error messages, the error messages are ignored.
- Not disconnected every minute.
- Disabled auto-connect when creating database.
- Removed filenames from error messages.

Test Plan: Run CQL & the new PGSQL test suite.

Reviewers: mihnea, robert

Reviewed By: robert

Subscribers: mikhail, karthik, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D4672

.arclint

@@ -1,5 +1,11 @@
 {
-  "exclude":"(\\.l$|Makefile$|src/yb/common/ql_type.h$)",
+  "exclude": [
+     "(\\.l$)",
+     "(Makefile$)",
+     "(src/yb/common/ql_type.h$)",
+     "(\\.sql$)",
+     "(\\.txt$)"
+  ],
   "linters": {
     "text": {
       "type": "text",

src/yb/integration-tests/external_mini_cluster.cc

@@ -32,6 +32,8 @@
 
 #include "yb/integration-tests/external_mini_cluster.h"
 
+#include <sys/stat.h>
+
 #include <atomic>
 #include <memory>
 #include <mutex>
@@ -76,7 +78,7 @@
 #include "yb/util/size_literals.h"
 
 using namespace std::literals;  // NOLINT
-using namespace yb::size_literals;
+using namespace yb::size_literals;  // NOLINT
 
 using std::atomic;
 using std::lock_guard;
@@ -202,6 +204,17 @@ Status ExternalMiniCluster::HandleOptions() {
   if (data_root_.empty()) {
     // If they don't specify a data root, use the current gtest directory.
     data_root_ = JoinPathSegments(GetTestDataDirectory(), "minicluster-data");
+
+    // If "data_root_counter" is non-negative, and the auto-generated "data_root_" directory already
+    // exists, create a subdirectory using the counter value as its name. The caller should maintain
+    // this counter and increment it for each test run.
+    if (opts_.data_root_counter >= 0) {
+      struct stat sb;
+      if (stat(data_root_.c_str(), &sb) == 0 && S_ISDIR(sb.st_mode)) {
+        data_root_ = Substitute("$0/$1", data_root_, opts_.data_root_counter);
+        CHECK_EQ(mkdir(data_root_.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH), 0);
+      }
+    }
   }
 
   return Status::OK();
@@ -1059,6 +1072,11 @@ vector<ExternalDaemon*> ExternalMiniCluster::daemons() const {
   return results;
 }
 
+HostPort ExternalMiniCluster::pgsql_hostport(int node_index) const {
+  return HostPort(tablet_servers_[node_index]->bind_host(),
+                  tablet_servers_[node_index]->pgsql_rpc_port());
+}
+
 std::shared_ptr<rpc::Messenger> ExternalMiniCluster::messenger() {
   return messenger_;
 }

src/yb/integration-tests/external_mini_cluster.h

@@ -89,6 +89,14 @@ struct ExternalMiniClusterOptions {
   // Default: "", which auto-generates a unique path for this cluster.
   std::string data_root;
 
+  // Set data_root_counter to non-negative number if your test run need to create an new and empty
+  // cluster every time ExternalMiniCluster() is constructed.
+  // - During a test run, data_root will stay the same until the run is finished, so recreating a
+  //   brand new cluster from scratch is not possible because the database location stays the same.
+  // - When data_root_counter is non-negative, a new "data_root" is generated every time
+  //   ExternalMiniCluster() is constructed.
+  int data_root_counter = -1;
+
   // If true, binds each tablet server to a different loopback address.  This affects the server's
   // RPC server, and also forces the server to only use this IP address for outgoing socket
   // connections as well.  This allows the use of iptables on the localhost to simulate network
@@ -251,6 +259,9 @@ class ExternalMiniCluster : public MiniClusterBase {
   // Return all tablet servers and masters.
   std::vector<ExternalDaemon*> daemons() const;
 
+  // Get tablet server host.
+  HostPort pgsql_hostport(int node_index) const;
+
   int num_tablet_servers() const {
     return tablet_servers_.size();
   }
@@ -557,6 +568,14 @@ class ExternalTabletServer : public ExternalDaemon {
   // Restarts the daemon. Requires that it has previously been shutdown.
   CHECKED_STATUS Restart(bool start_cql_proxy = true);
 
+  // Postgres addresses.
+  const string& bind_host() const {
+    return bind_host_;
+  }
+  uint16_t pgsql_rpc_port() const {
+    return pgsql_rpc_port_;
+  }
+
  protected:
   CHECKED_STATUS DeleteServerInfoPaths() override;
 

src/yb/yql/pgsql/CMakeLists.txt

@@ -33,6 +33,9 @@ add_subdirectory(pbgen)
 # Code execution.
 add_subdirectory(pbexec)
 
-#utility
+# Utility.
 add_subdirectory(util)
 add_subdirectory(ybpostgres)
+
+# Test.
+add_subdirectory(test)

src/yb/yql/pgsql/pbexec/pg_exec_context.cc

@@ -42,7 +42,9 @@ CHECKED_STATUS PgExecContext::Apply(std::shared_ptr<client::YBPgsqlOp> op) {
 CHECKED_STATUS PgExecContext::CreateDatabase(const std::string& db_name) {
   CHECK(pg_env_) << "Execution env is not defined";
   RETURN_NOT_OK(pg_env_->CreateDatabase(db_name));
-  pg_session_->set_current_database(db_name);
+
+  // Remove auto-connect during creation.
+  // pg_session_->set_current_database(db_name);
   return Status::OK();
 }
 

src/yb/yql/pgsql/ptree/pg_process_context.cc

@@ -206,7 +206,7 @@ CHECKED_STATUS PgProcessContext::Error(const TreeNode *tnode,
 CHECKED_STATUS PgProcessContext::Error(const TreeNode *tnode,
                                          const Status& s,
                                          ErrorCode error_code) {
-  return Error(tnode->loc(), s.ToString().c_str(), error_code);
+  return Error(tnode->loc(), s.ToUserMessage().c_str(), error_code);
 }
 
 CHECKED_STATUS PgProcessContext::Error(const TreeNode::SharedPtr& tnode,
@@ -223,7 +223,7 @@ CHECKED_STATUS PgProcessContext::Error(const TreeNode::SharedPtr& tnode,
 CHECKED_STATUS PgProcessContext::Error(const TreeNode::SharedPtr& tnode,
                                          const Status& s,
                                          ErrorCode error_code) {
-  return Error(tnode->loc(), s.ToString().c_str(), error_code);
+  return Error(tnode->loc(), s.ToUserMessage().c_str(), error_code);
 }
 
 }  // namespace pgsql

src/yb/yql/pgsql/ptree/pg_tbcall.cc

@@ -106,7 +106,7 @@ CHECKED_STATUS PgTBcall::Analyze(PgCompileContext *compile_context) {
       got_first_arg = true;
     }
     err_msg += ")'. ";
-    err_msg += s.ToString();
+    err_msg += s.ToUserMessage();
     LOG(INFO) << err_msg;
     return compile_context->Error(this, err_msg.c_str(), ErrorCode::INVALID_FUNCTION_CALL);
   }
@@ -256,7 +256,7 @@ CHECKED_STATUS PgTBcall::Analyze(PgCompileContext *compile_context) {
       if (!s.ok()) {
         LOG(ERROR) << "Arguments to builtin call " << name_->c_str() << "() is compatible with "
                    << "its signature but converting the argument to the desired type failed";
-        string err_msg = (s.ToString() + "CAST " + expr->ql_type()->ToString() + " AS " +
+        string err_msg = (s.ToUserMessage() + "CAST " + expr->ql_type()->ToString() + " AS " +
             QLType::ToCQLString(formal_types[pindex]) + " failed");
         return compile_context->Error(this, err_msg.c_str(), ErrorCode::INVALID_FUNCTION_CALL);
       }

src/yb/yql/pgsql/server/pg_rpc.cc

@@ -332,7 +332,7 @@ void PgInboundCall::RespondFailure(rpc::ErrorStatusPB::RpcErrorCodePB error_code
                                    const Status& status) {
   // Write error message.
   StringInfo pg_msg;
-  conn_context_->pgsend().WriteErrorMessage(-1, status.ToString(), &pg_msg);
+  conn_context_->pgsend().WriteErrorMessage(-1, status.ToUserMessage(), &pg_msg);
   Respond(pg_msg->data, false);
 }
 

src/yb/yql/pgsql/server/pg_server_options.cc

@@ -28,7 +28,7 @@ namespace pgserver {
 PgServerOptions::PgServerOptions() {
   server_type = "tserver";
   rpc_opts.default_port = PgServer::kDefaultPort;
-  // rpc_opts.connection_keepalive_time_ms = FLAGS_pgsql_rpc_keepalive_time_ms;
+  rpc_opts.connection_keepalive_time_ms = FLAGS_pgsql_rpc_keepalive_time_ms;
 }
 
 } // namespace pgserver

src/yb/yql/pgsql/syn/pg_gram.y

@@ -2767,8 +2767,13 @@ opt_ordinality:
 ;
 
 opt_where_clause:
-  /*EMPTY*/                     { $$ = nullptr; }
-  | where_clause                { $$ = $1; }
+  /*EMPTY*/ {
+    $$ = nullptr;
+  }
+  | where_clause {
+    PARSER_UNSUPPORTED(@1);
+    $$ = $1;
+  }
 ;
 
 where_clause:

src/yb/yql/pgsql/test/CMakeLists.txt

@@ -0,0 +1,24 @@
+# Copyright (c) YugaByte, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+# in compliance with the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software distributed under the License
+# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+# or implied.  See the License for the specific language governing permissions and limitations
+# under the License.
+#
+
+add_library(pgsql_test pg_psql_test.cc)
+target_link_libraries(pgsql_test
+                      yb-pgsql
+                      yb_client
+                      yb_client_test_util
+                      integration-tests)
+add_dependencies(pgsql_test yb-pgserver)
+
+set(YB_TEST_LINK_LIBS pgsql_test ${YB_MIN_TEST_LIBS})
+ADD_YB_TEST(pg_run_test)
+ADD_COMMON_YB_TEST_DEPENDENCIES(pg_run_test)

src/yb/yql/pgsql/test/basic/create_table.sql

@@ -0,0 +1,19 @@
+CREATE DATABASE mytest_on_create_table;
+\c mytest_on_create_table;
+
+CREATE TABLE tab_without_key(id INT, name VARCHAR);
+
+CREATE TABLE tab_with_one_col_key(id INT PRIMARY KEY, name VARCHAR);
+
+CREATE TABLE tab_with_multi_col_key(id INT,
+                                    name VARCHAR,
+                                    salary DOUBLE PRECISION, primary key(id, name));
+
+DROP TABLE tab_without_key;
+
+DROP TABLE tab_with_one_col_key;
+
+DROP TABLE tab_with_multi_col_key;
+
+\c pgsql_testdb;
+DROP DATABASE mytest_on_create_table;

src/yb/yql/pgsql/test/basic/insert.sql

@@ -0,0 +1,48 @@
+CREATE DATABASE mytest_on_insert;
+\c mytest_on_insert;
+
+CREATE TABLE tab_without_key(id INT, name VARCHAR);
+
+INSERT INTO tab_without_key(id) VALUES(1);
+INSERT INTO tab_without_key(name) VALUES('one');
+INSERT INTO tab_without_key(id, name) VALUES(2, 'two');
+INSERT INTO tab_without_key VALUES(1, 'one');
+
+SELECT id FROM tab_without_key;
+SELECT name FROM tab_without_key;
+SELECT id, name FROM tab_without_key;
+SELECT * FROM tab_without_key;
+
+CREATE TABLE tab_with_one_col_key(id INT PRIMARY KEY, name VARCHAR);
+
+INSERT INTO tab_with_one_col_key(id) VALUES(1);
+INSERT INTO tab_with_one_col_key(name) VALUES('one');
+INSERT INTO tab_with_one_col_key(id, name) VALUES(2, 'two');
+INSERT INTO tab_with_one_col_key VALUES(1, 'one');
+
+SELECT id FROM tab_with_one_col_key;
+SELECT name FROM tab_with_one_col_key;
+SELECT id, name FROM tab_with_one_col_key;
+SELECT * FROM tab_with_one_col_key;
+
+CREATE TABLE tab_with_multi_col_key(id INT,
+                                    name VARCHAR,
+                                    salary DOUBLE PRECISION, primary key(id, name));
+
+INSERT INTO tab_with_multi_col_key(id) VALUES(1);
+INSERT INTO tab_with_multi_col_key(name) VALUES('one');
+INSERT INTO tab_with_multi_col_key(id, name) VALUES(2, 'two');
+INSERT INTO tab_with_multi_col_key VALUES(3, 'three', 999.99);
+
+SELECT id FROM tab_with_multi_col_key;
+SELECT name FROM tab_with_multi_col_key;
+SELECT id, name FROM tab_with_multi_col_key;
+SELECT id, name, salary FROM tab_with_multi_col_key;
+SELECT * FROM tab_with_multi_key;
+
+DROP TABLE tab_without_key;
+DROP TABLE tab_with_one_col_key;
+DROP TABLE tab_with_multi_col_key;
+
+\c pgsql_testdb;
+DROP DATABASE mytest_on_insert;

src/yb/yql/pgsql/test/basic/result/create_table.txt

@@ -0,0 +1,8 @@
+DATABASE CREATED
+TABLE CREATED
+TABLE CREATED
+TABLE CREATED
+TABLE DROPPED
+TABLE DROPPED
+TABLE DROPPED
+DATABASE DROPPED

src/yb/yql/pgsql/test/basic/result/insert.txt

@@ -0,0 +1,97 @@
+DATABASE CREATED
+TABLE CREATED
+INSERT 0 1
+INSERT 0 1
+INSERT 0 1
+INSERT 0 1
+ id 
+----
+  1
+
+  2
+  1
+(4 rows)
+
+ name 
+------
+
+ one
+ two
+ one
+(4 rows)
+
+ id | name 
+----+------
+  1 | 
+    | one
+  2 | two
+  1 | one
+(4 rows)
+
+ id | name 
+----+------
+  1 | 
+    | one
+  2 | two
+  1 | one
+(4 rows)
+
+TABLE CREATED
+INSERT 0 1
+INSERT 0 1
+INSERT 0 1
+ id 
+----
+  1
+  2
+(2 rows)
+
+ name 
+------
+ one
+ two
+(2 rows)
+
+ id | name 
+----+------
+  1 | one
+  2 | two
+(2 rows)
+
+ id | name 
+----+------
+  1 | one
+  2 | two
+(2 rows)
+
+TABLE CREATED
+INSERT 0 1
+INSERT 0 1
+ id 
+----
+  2
+  3
+(2 rows)
+
+ name  
+-------
+ two
+ three
+(2 rows)
+
+ id | name  
+----+-------
+  2 | two
+  3 | three
+(2 rows)
+
+ id | name  |   salary   
+----+-------+------------
+  2 | two   |           
+  3 | three | 999.990000
+(2 rows)
+
+TABLE DROPPED
+TABLE DROPPED
+TABLE DROPPED
+DATABASE DROPPED