Skip to content

Commit

Permalink
clean error msgs
Browse files Browse the repository at this point in the history 
* Clean up exception/error message construction
* Update Tests verification to reflect exception message changes
* Rebase
  • Loading branch information
@danielligman @andrewseidl
danielligman authored and andrewseidl committed Jul 19, 2021
1 parent 731ac89 commit 98b0772
Show file tree
Hide file tree
Showing 25 changed files with 264 additions and 287 deletions.
4 changes: 2 additions & 2 deletions Catalog/Catalog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4645,8 +4645,8 @@ std::string Catalog::dumpCreateTable(const TableDescriptor* td,
os << (ti.get_notnull() ? " NOT NULL" : "");
if (shared_dict_column_names.find(cd->columnName) ==
shared_dict_column_names.end()) {
// avoids "Exception: Column ... shouldn't specify an encoding, it borrows it
// from the referenced column"
// avoids "Column ... shouldn't specify an encoding, it borrows it
// from the referenced column"
if (ti.is_string() || (ti.is_array() && ti.get_subtype() == kTEXT)) {
auto size = ti.is_array() ? ti.get_logical_size() : ti.get_size();
if (ti.get_compression() == kENCODING_DICT) {
Expand Down
2 changes: 1 addition & 1 deletion Embedded/EmbeddedDbFSITest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ dropoff_puma BIGINT) WITH (storage_type='CSV:") + csv_path + std::string("', fra
}
}
} catch (std::exception& e) {
std::cerr << "Exception: " << e.what() << "\n";
std::cerr << e.what() << "\n";
}
return 0;
}
2 changes: 1 addition & 1 deletion Embedded/EmbeddedDbTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ int main(int argc, char* argv[]) {
}
}
} catch (std::exception& e) {
std::cerr << "Exception: " << e.what() << "\n";
std::cerr << e.what() << "\n";
}
return 0;
}
3 changes: 1 addition & 2 deletions LockMgr/LockMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ ChunkKey chunk_key_for_table(const Catalog_Namespace::Catalog& cat,
return chunk_key;
} else {
throw std::runtime_error("Table/View " + tableName + " for catalog " +
cat.getCurrentDB().dbName +
" does not exist, could not generate chunk key");
cat.getCurrentDB().dbName + " does not exist");
}
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/AlterTableDdlTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ TEST_F(AlterTableSetMaxRowsTest, NegativeMaxRows) {
sqlAndCompareResult("select * from test_table;",
{{i(1)}, {i(2)}, {i(3)}, {i(4)}, {i(5)}});
queryAndAssertException("alter table test_table set max_rows = -1;",
"Exception: Max rows cannot be a negative number.");
"Max rows cannot be a negative number.");
assertMaxRows(DEFAULT_MAX_ROWS);
sqlAndCompareResult("select * from test_table;",
{{i(1)}, {i(2)}, {i(3)}, {i(4)}, {i(5)}});
Expand Down
121 changes: 57 additions & 64 deletions Tests/CreateAndDropTableDdlTest.cpp
View file

Large diffs are not rendered by default.

46 changes: 22 additions & 24 deletions Tests/DBObjectPrivilegesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ class InvalidGrantSyntax : public DBHandlerTestFixture {};

TEST_F(InvalidGrantSyntax, InvalidGrantSyntax) {
std::string error_message;
error_message = "Exception: Syntax error at: ON";
error_message = "Syntax error at: ON";

queryAndAssertException("GRANT SELECT, INSERT, ON TABLE tbl TO Arsenal, Juventus;",
error_message);
Expand Down Expand Up @@ -3151,7 +3151,7 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeDropPrivilege) {
std::string privilege{"DROP"};
std::string query{"DROP FOREIGN TABLE test_table;"};
std::string no_privilege_exception{
"Exception: Foreign table \"test_table\" will not be dropped. User has no DROP "
"Foreign table \"test_table\" will not be dropped. User has no DROP "
"TABLE privileges."};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3164,7 +3164,7 @@ TEST_F(TablePermissionsTest, TableGrantRevokeDropPrivilege) {
std::string privilege{"DROP"};
std::string query{"DROP TABLE test_table;"};
std::string no_privilege_exception{
"Exception: Table test_table will not be dropped. User has no proper privileges."};
"Table test_table will not be dropped. User has no proper privileges."};
createTestTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
grantThenRevokePrivilegeToTestUser(privilege);
Expand Down Expand Up @@ -3195,11 +3195,11 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeDeletePrivilege) {
std::string privilege{"DELETE"};
std::string query{"DELETE FROM test_table WHERE i = 1;"};
std::string no_privilege_exception{
"Exception: Violation of access privileges: user test_user has no proper "
"Violation of access privileges: user test_user has no proper "
"privileges for "
"object test_table"};
std::string query_exception{
"Exception: DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"supported for foreign tables."};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3212,7 +3212,7 @@ TEST_F(TablePermissionsTest, TableGrantRevokeDeletePrivilege) {
std::string privilege{"DELETE"};
std::string query{"DELETE FROM test_table WHERE i = 1;"};
std::string no_privilege_exception{
"Exception: Violation of access privileges: user test_user has no proper "
"Violation of access privileges: user test_user has no proper "
"privileges for "
"object test_table"};
createTestTable();
Expand All @@ -3225,10 +3225,9 @@ TEST_F(TablePermissionsTest, TableGrantRevokeDeletePrivilege) {
TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeInsertPrivilege) {
std::string privilege{"INSERT"};
std::string query{"INSERT INTO test_table VALUES (2);"};
std::string no_privilege_exception{
"Exception: User has no insert privileges on test_table."};
std::string no_privilege_exception{"User has no insert privileges on test_table."};
std::string query_exception{
"Exception: DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"supported for foreign tables."};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3240,8 +3239,7 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeInsertPrivilege) {
TEST_F(TablePermissionsTest, TableGrantRevokeInsertPrivilege) {
std::string privilege{"INSERT"};
std::string query{"INSERT INTO test_table VALUES (2);"};
std::string no_privilege_exception{
"Exception: User has no insert privileges on test_table."};
std::string no_privilege_exception{"User has no insert privileges on test_table."};
createTestTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
grantThenRevokePrivilegeToTestUser(privilege);
Expand All @@ -3253,10 +3251,10 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeTruncatePrivilege) {
std::string privilege{"TRUNCATE"};
std::string query{"TRUNCATE TABLE test_table;"};
std::string no_privilege_exception{
"Exception: Table test_table will not be truncated. User test_user has no proper "
"Table test_table will not be truncated. User test_user has no proper "
"privileges."};
std::string query_exception{
"Exception: DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"supported for foreign tables."};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3269,7 +3267,7 @@ TEST_F(TablePermissionsTest, TableGrantRevokeTruncatePrivilege) {
std::string privilege{"TRUNCATE"};
std::string query{"TRUNCATE TABLE test_table;"};
std::string no_privilege_exception{
"Exception: Table test_table will not be truncated. User test_user has no proper "
"Table test_table will not be truncated. User test_user has no proper "
"privileges."};
createTestTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3282,11 +3280,11 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeUpdatePrivilege) {
std::string privilege{"UPDATE"};
std::string query{"UPDATE test_table SET i = 2 WHERE i = 1;"};
std::string no_privilege_exception{
"Exception: Violation of access privileges: user test_user has no proper "
"Violation of access privileges: user test_user has no proper "
"privileges for "
"object test_table"};
std::string query_exception{
"Exception: DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"DELETE, INSERT, TRUNCATE, OR UPDATE commands are not "
"supported for foreign tables."};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3299,7 +3297,7 @@ TEST_F(TablePermissionsTest, TableGrantRevokeUpdatePrivilege) {
std::string privilege{"UPDATE"};
std::string query{"UPDATE test_table SET i = 2 WHERE i = 1;"};
std::string no_privilege_exception{
"Exception: Violation of access privileges: user test_user has no proper "
"Violation of access privileges: user test_user has no proper "
"privileges for "
"object test_table"};
createTestTable();
Expand All @@ -3316,7 +3314,7 @@ TEST_P(ForeignTableAndTablePermissionsTest, GrantRevokeShowCreateTablePrivilege)
}
std::string privilege{"DROP"};
std::string query{"SHOW CREATE TABLE test_table;"};
std::string no_privilege_exception{"Exception: Table/View test_table does not exist."};
std::string no_privilege_exception{"Table/View test_table does not exist."};
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
grantThenRevokePrivilegeToTestUser(privilege);
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3327,7 +3325,7 @@ TEST_F(TablePermissionsTest, TableGrantRevokeAlterTablePrivilege) {
std::string privilege{"ALTER"};
std::string query{"ALTER TABLE test_table RENAME COLUMN i TO j;"};
std::string no_privilege_exception{
"Exception: Current user does not have the privilege to alter table: test_table"};
"Current user does not have the privilege to alter table: test_table"};
createTestTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
grantThenRevokePrivilegeToTestUser(privilege);
Expand All @@ -3340,7 +3338,7 @@ TEST_F(ForeignTablePermissionsTest, TableGrantRevokeAlterForeignTablePrivilege)
std::string query{
"ALTER FOREIGN TABLE test_table SET (refresh_update_type = 'append');"};
std::string no_privilege_exception{
"Exception: Current user does not have the privilege to alter foreign table: "
"Current user does not have the privilege to alter foreign table: "
"test_table"};
createTestForeignTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
Expand All @@ -3353,7 +3351,7 @@ TEST_F(TablePermissionsTest, TableRenameTablePrivilege) {
std::string privilege{"ALTER"};
std::string query{"RENAME TABLE test_table TO renamed_test_table;"};
std::string no_privilege_exception{
"Exception: Current user does not have the privilege to alter table: test_table"};
"Current user does not have the privilege to alter table: test_table"};
createTestTable();
queryAsTestUserWithNoPrivilegeAndAssertException(query, no_privilege_exception);
queryAsTestUserWithPrivilege(query, privilege);
Expand Down Expand Up @@ -3391,15 +3389,15 @@ TEST_F(TablePermissionsTest, TableAllPrivileges) {
TEST_F(ForeignTablePermissionsTest, ForeignTableGrantRevokeCreateTablePrivilege) {
login("test_user", "test_pass");
executeLambdaAndAssertException([this] { createTestForeignTable(); },
"Exception: Foreign table \"test_table\" will not be "
"Foreign table \"test_table\" will not be "
"created. User has no CREATE TABLE privileges.");

switchToAdmin();
sql("GRANT CREATE TABLE ON DATABASE omnisci TO test_user;");
sql("REVOKE CREATE TABLE ON DATABASE omnisci FROM test_user;");
login("test_user", "test_pass");
executeLambdaAndAssertException([this] { createTestForeignTable(); },
"Exception: Foreign table \"test_table\" will not be "
"Foreign table \"test_table\" will not be "
"created. User has no CREATE TABLE privileges.");

switchToAdmin();
Expand Down Expand Up @@ -3438,7 +3436,7 @@ TEST_F(ForeignTablePermissionsTest, ForeignTableRefreshNonOwner) {
login("test_user", "test_pass");
runQueryAndAssertException(
"REFRESH FOREIGN TABLES test_table;",
"Exception: REFRESH FOREIGN TABLES failed on table \"test_table\". It can only be "
"REFRESH FOREIGN TABLES failed on table \"test_table\". It can only be "
"executed by super user or owner of the object.");
}

Expand Down
8 changes: 4 additions & 4 deletions Tests/DashboardAndCustomExpressionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ TEST_P(ShareAndUnshareDashboardsTest, NonExistentGroupOrUser) {
[&, this] {
shareOrUnshareDashboards(permissions, {"non_existent_user"}, do_share);
},
"Exception: User/Role 'non_existent_user' does not exist");
"User/Role 'non_existent_user' does not exist");
}

TEST_P(ShareAndUnshareDashboardsTest, NonExistentDashboards) {
Expand Down Expand Up @@ -807,7 +807,7 @@ TEST_F(DashboardBulkDeleteTest, SomeInvalidIDs) {
FAIL() << "An exception should have been thrown for this test case.";
} catch (const TOmniSciException& e) {
assertExceptionMessage(e,
"Exception: Delete dashboard(s) failed with "
"Delete dashboard(s) failed with "
"error(s):\nDashboard id: 0 - Dashboard id does not exist");
}
ASSERT_EQ(getNumDashboards(), size_t(1));
Expand All @@ -830,7 +830,7 @@ TEST_F(DashboardBulkDeleteTest, NoDeleteDashboardPrivilege) {
} catch (const TOmniSciException& e) {
assertExceptionMessage(
e,
"Exception: Delete dashboard(s) failed with error(s):\nDashboard id: " +
"Delete dashboard(s) failed with error(s):\nDashboard id: " +
std::to_string(db_id_2) +
" - User should be either owner of dashboard or super user to delete it");
}
Expand Down Expand Up @@ -867,7 +867,7 @@ TEST_F(DashboardBulkDeleteTest, InvalidNoPrivilegeMix) {
} catch (const TOmniSciException& e) {
assertExceptionMessage(
e,
std::string("Exception: Delete dashboard(s) failed with error(s):\n") +
std::string("Delete dashboard(s) failed with error(s):\n") +
"Dashboard id: 0 - Dashboard id does not exist\n" +
"Dashboard id: " + std::to_string(db_id) +
" - User should be either owner of dashboard or super user to delete it");
Expand Down
21 changes: 10 additions & 11 deletions Tests/ExecuteTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2343,9 +2343,9 @@ TEST(Select, CountDistinct) {
c("SELECT COUNT(distinct x) FROM test;", dt);
c("SELECT COUNT(distinct b) FROM test;", dt);
THROW_ON_AGGREGATOR(c("SELECT COUNT(distinct f) FROM test;",
dt)); // Exception: Cannot use a fast path for COUNT distinct
dt)); // Cannot use a fast path for COUNT distinct
THROW_ON_AGGREGATOR(c("SELECT COUNT(distinct d) FROM test;",
dt)); // Exception: Cannot use a fast path for COUNT distinct
dt)); // Cannot use a fast path for COUNT distinct
c("SELECT COUNT(distinct str) FROM test;", dt);
c("SELECT COUNT(distinct ss) FROM test;", dt);
c("SELECT COUNT(distinct x + 1) FROM test;", dt);
Expand All @@ -2362,10 +2362,10 @@ TEST(Select, CountDistinct) {
c("SELECT AVG(z), COUNT(distinct x) AS dx FROM test GROUP BY y HAVING dx > 1;", dt);
THROW_ON_AGGREGATOR(
c("SELECT z, str, COUNT(distinct f) FROM test GROUP BY z, str ORDER BY str DESC;",
dt)); // Exception: Cannot use a fast path for COUNT distinct
dt)); // Cannot use a fast path for COUNT distinct
c("SELECT COUNT(distinct x * (50000 - 1)) FROM test;", dt);
EXPECT_THROW(run_multiple_agg("SELECT COUNT(distinct real_str) FROM test;", dt),
std::runtime_error); // Exception: Strings must be dictionary-encoded
std::runtime_error); // Strings must be dictionary-encoded
// for COUNT(DISTINCT).
}
}
Expand Down Expand Up @@ -2497,7 +2497,7 @@ TEST(Select, ApproxMedianSanity) {
} catch (std::runtime_error const& e) {
EXPECT_EQ(
std::string(e.what()),
"TException - service has thrown: TOmniSciException(error_msg=Exception: "
"TException - service has thrown: TOmniSciException(error_msg="
"APPROX_QUANTILE/MEDIAN is not supported in distributed mode at this time.)");
} catch (...) {
EXPECT_TRUE(false) << "std::runtime_error expected for approx_median query.";
Expand Down Expand Up @@ -3082,7 +3082,7 @@ TEST(Select, Case) {
};
EXPECT_ANY_THROW(run_multiple_agg(
R"(SELECT CASE WHEN x = 7 THEN 'a' WHEN x = 8 then str ELSE fixed_str END FROM test;)",
dt)); // Exception: Cast from dictionary-encoded string to none-encoded would
dt)); // Cast from dictionary-encoded string to none-encoded would
// be slow
g_enable_watchdog = false;
// casts not yet supported in distributed mode
Expand Down Expand Up @@ -8566,7 +8566,7 @@ TEST(Select, Export_Via_Query_Having_Scalar_Subquery) {
// EXPORT stmt needs "validation_query" to gather some info from the query
// before doing the actual data export
// Here, if we do export via custom query having scalar subquery,
// we throw "Exception: Scalar sub-query returned no results"
// we throw "Scalar sub-query returned no results"
// since RexSubquery Analyzer does not know about the validation query
// so we have to let subquery analyzer know about that we do process validation query
// and keep doing processing instead of throwing the exception
Expand Down Expand Up @@ -19747,10 +19747,9 @@ TEST(Select, ParseIntegerExceptions) {
EXPECT_TRUE(false) << "Exception expected for query: " << test.query;
} catch (std::runtime_error const& e) {
if (g_aggregator) {
EXPECT_EQ(
e.what(),
"TException - service has thrown: TOmniSciException(error_msg=Exception: " +
test.exception + ')');
EXPECT_EQ(e.what(),
"TException - service has thrown: TOmniSciException(error_msg=" +
test.exception + ')');
} else {
EXPECT_EQ(e.what(), test.exception);
}
Expand Down
Loading

0 comments on commit 98b0772

Please sign in to comment.