Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MCOL-5890: DROP TABLE IF EXISTS should not generate errors #3411

Draft
wants to merge 1 commit into
base: stable-23.10
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions dbcon/ddlpackage/ddl.y
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/* $Id: ddl.y 9314 2013-03-19 17:39:25Z dhall $ */
/* This describes a substantial subset of SQL92 DDL with some
enhancements from various vendors. Most of the nomenclature in the
grammar is drawn from SQL92.
grammar is drawn from SQL92.

If you have to care about this grammar, you should consult one or
more of the following sources:
Expand Down Expand Up @@ -50,7 +50,7 @@
#define scanner x->scanner

using namespace std;
using namespace ddlpackage;
using namespace ddlpackage;

int ddllex(YYSTYPE* ddllval, void* yyscanner);
void ddlerror(struct pass_to_bison* x, char const *s);
Expand Down Expand Up @@ -101,7 +101,7 @@ void fix_column_length_and_charset(SchemaObject* elem, const CHARSET_INFO* def_c
column->fType->fLength = 255;
else if (column->fType->fLength <= 65535)
column->fType->fLength = 65535;
else
else
column->fType->fLength = 16777215;
}
}
Expand Down Expand Up @@ -244,7 +244,7 @@ ZEROFILL
%type <columnType> approximate_numeric_type
%type <str> opt_display_width
%type <str> opt_display_precision_scale_null
%type <str> opt_if_exists
%type <flag> opt_if_exists
%type <str> opt_if_not_exists
%type <str> opt_signed
%type <str> opt_zerofill
Expand Down Expand Up @@ -289,7 +289,7 @@ stmtmulti:
}
}
| stmt
{
{
if ($1 != NULL)
{
$$ = x->fParseTree;
Expand Down Expand Up @@ -317,19 +317,19 @@ drop_table_statement:
DROP TABLE opt_if_exists qualified_name {$$ = new DropTableStatement($4, false);}
| DROP TABLE opt_if_exists qualified_name CASCADE CONSTRAINTS
{
{$$ = new DropTableStatement($4, true);}
{$$ = new DropTableStatement($4, true, $3);}
}
;

opt_if_exists:
IDB_IF EXISTS {$$ = NULL;}
IDB_IF EXISTS {$$ = true;}
| {$$ = NULL;}
;

drop_index_statement:
DROP INDEX qualified_name {$$ = new DropIndexStatement($3);}
;

/* Notice that we allow table_options here (e.g. engine=infinidb) but
we ignore them. */
create_index_statement:
Expand Down Expand Up @@ -374,7 +374,7 @@ trunc_table_statement:
rename_table_statement:
RENAME TABLE qualified_name TO qualified_name
{
// MCOL-876. The change reuses existing infrastructure.
// MCOL-876. The change reuses existing infrastructure.
AtaRenameTable* renameAction = new AtaRenameTable($5);
AlterTableActionList* actionList = new AlterTableActionList();
actionList->push_back(renameAction);
Expand Down Expand Up @@ -526,7 +526,7 @@ table_options:
$$ = new TableOptionMap();
(*$$)[$1->first] = $1->second;
delete $1;
}
}
| table_options table_option
{
$$ = $1;
Expand Down Expand Up @@ -589,7 +589,7 @@ alter_table_actions:
/* An alter_table_statement requires at least one action.
So, this shouldn't happen. */
$$ = NULL;
}
}
}
| alter_table_actions ',' alter_table_action
{
Expand Down Expand Up @@ -651,30 +651,30 @@ rename_column:
{$$ = new AtaRenameColumn($2, $3, $4, $5, NULL, $6);}
|CHANGE COLUMN column_name column_name data_type column_qualifier_list column_option
{$$ = new AtaRenameColumn($3, $4, $5, $6, NULL, $7);}
|CHANGE column_name column_name data_type default_clause
|CHANGE column_name column_name data_type default_clause
{$$ = new AtaRenameColumn($2, $3, $4, NULL, $5);}
|CHANGE COLUMN column_name column_name data_type default_clause
{$$ = new AtaRenameColumn($3, $4, $5, NULL, $6);}
|CHANGE column_name column_name data_type default_clause column_option
{$$ = new AtaRenameColumn($2, $3, $4, NULL, $5, $6);}
|CHANGE COLUMN column_name column_name data_type default_clause column_option
{$$ = new AtaRenameColumn($3, $4, $5, NULL, $6, $7);}
|CHANGE column_name column_name data_type column_qualifier_list default_clause
|CHANGE column_name column_name data_type column_qualifier_list default_clause
{$$ = new AtaRenameColumn($2, $3, $4, $5, $6);}
|CHANGE COLUMN column_name column_name data_type column_qualifier_list default_clause
{$$ = new AtaRenameColumn($3, $4, $5, $6, $7);}
|CHANGE column_name column_name data_type default_clause column_qualifier_list
|CHANGE column_name column_name data_type default_clause column_qualifier_list
{$$ = new AtaRenameColumn($2, $3, $4, $6, $5);}
|CHANGE COLUMN column_name column_name data_type default_clause column_qualifier_list
{$$ = new AtaRenameColumn($3, $4, $5, $7, $6);}
|CHANGE column_name column_name data_type column_qualifier_list default_clause column_option
{$$ = new AtaRenameColumn($2, $3, $4, $5, $6, $7);}
|CHANGE COLUMN column_name column_name data_type column_qualifier_list default_clause column_option
{$$ = new AtaRenameColumn($3, $4, $5, $6, $7, $8);}
{$$ = new AtaRenameColumn($3, $4, $5, $6, $7, $8);}
|CHANGE column_name column_name data_type default_clause column_qualifier_list column_option
{$$ = new AtaRenameColumn($2, $3, $4, $6, $5, $7);}
|CHANGE COLUMN column_name column_name data_type default_clause column_qualifier_list column_option
{$$ = new AtaRenameColumn($3, $4, $5, $7, $6, $8);}
{$$ = new AtaRenameColumn($3, $4, $5, $7, $6, $8);}
;

drop_table_constraint_def:
Expand Down Expand Up @@ -747,8 +747,8 @@ constraint_name:

column_option:
COMMENT string_literal {$$ = $2;}
column_def:

column_def:
column_name data_type opt_null_tok
{
$$ = new ColumnDef($1, $2, NULL, NULL );
Expand Down Expand Up @@ -863,7 +863,7 @@ column_qualifier_list:
{
$$ = new ColumnConstraintList();
$$->push_back($1);
}
}
| column_qualifier_list column_constraint_def
{
$$ = $1;
Expand Down Expand Up @@ -893,7 +893,7 @@ column_constraint_def:
$3->fDeferrable = $4->fDeferrable;
$3->fCheckTime = $4->fCheckTime;
}

}
;

Expand Down Expand Up @@ -956,7 +956,7 @@ check_constraint_def:

string_literal:
'\'' SCONST '\'' {$$ = $2;}
;
;

opt_quoted_literal:
string_literal
Expand Down Expand Up @@ -1251,7 +1251,7 @@ opt_signed:
'(' ICONST ',' ICONST ')' {$$ = NULL;}
| {$$ = NULL;}
;

literal:
ICONST
| string_literal
Expand Down
3 changes: 2 additions & 1 deletion dbcon/ddlpackage/ddlpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@ struct DropTableStatement : public SqlStatement
DropTableStatement() : fTableName(0)
{
}
EXPORT DropTableStatement(QualifiedName* qualifiedName, bool cascade);
EXPORT DropTableStatement(QualifiedName* qualifiedName, bool cascade, bool ifExists = false);

/** @brief Dump to stdout. */
EXPORT std::ostream& put(std::ostream& os) const;
Expand All @@ -1442,6 +1442,7 @@ struct DropTableStatement : public SqlStatement

QualifiedName* fTableName;
bool fCascade;
bool fIfExists;
};

/** @brief DebugStatement
Expand Down
7 changes: 3 additions & 4 deletions dbcon/ddlpackage/droptable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ using namespace std;

namespace ddlpackage
{
DropTableStatement::DropTableStatement(QualifiedName* qualifiedName, bool cascade)
: fTableName(qualifiedName), fCascade(cascade)
DropTableStatement::DropTableStatement(QualifiedName* qualifiedName, bool cascade, bool ifExists)
: fTableName(qualifiedName), fCascade(cascade), fIfExists(ifExists)
{
}

ostream& DropTableStatement::put(ostream& os) const
{
os << "Drop Table: " << *fTableName << " "
<< "C=" << fCascade << endl;
os << "Drop Table: " << *fTableName << " " << "C=" << fCascade << endl;
return os;
}

Expand Down
36 changes: 26 additions & 10 deletions dbcon/ddlpackageproc/droptableprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,20 +178,37 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackageInternal(ddlpack
if (ie.errorCode() == ERR_TABLE_NOT_IN_CATALOG)
{
Message::Args args;
Message message(1);
args.add("Table does not exist in ColumnStore.");
message.format(args);
result.result = DROP_TABLE_NOT_IN_CATALOG_ERROR;
result.message = message;
fSessionManager.rolledback(txnID);
return result;
args.add("Table ");
args.add(tableName.schema + "." + tableName.table);
args.add(" does not exist in ColumnStore.");

if (dropTableStmt->fIfExists) // Check if "IF EXISTS" is specified
{
Message message(1); // Informational log level
message.format(args);
result.result = NO_ERROR; // Success, no error
result.message = message;
fSessionManager.committed(txnID); // No action needed, commit
return result;
}
else
{
Message message(9); // Error log level
message.format(args);
result.result = DROP_TABLE_NOT_IN_CATALOG_ERROR;
result.message = message;
fSessionManager.rolledback(txnID);
return result;
}
}
else
{
result.result = DROP_ERROR;
Message::Args args;
Message message(9);
args.add("Drop table failed due to ");
args.add("Drop table ");
args.add(tableName.schema + "." + tableName.table);
args.add(" failed due to ");
args.add(ie.what());
message.format(args);
result.message = message;
Expand Down Expand Up @@ -517,8 +534,7 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackageInternal(ddlpack

if (rc != 0)
{
cout << txnID.id << " rolledback transaction "
<< " and valid is " << txnID.valid << endl;
cout << txnID.id << " rolledback transaction " << " and valid is " << txnID.valid << endl;
fSessionManager.rolledback(txnID);
}
else
Expand Down
16 changes: 16 additions & 0 deletions mysql-test/columnstore/bugfixes/mcol-5890.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
DROP DATABASE IF EXISTS MCOL5890;
CREATE DATABASE MCOL5890;
USE MCOL5890;
# Test 1: DROP TABLE IF EXISTS on non-existent table
DROP TABLE IF EXISTS test.does_not_exist;
Warnings:
Note 1051 Unknown table 'test.does_not_exist'
# Test 2: DROP TABLE on non-existent table
DROP TABLE test.does_not_exist;
ERROR 42S02: Unknown table 'test.does_not_exist'
# Test 3: DROP TABLE IF EXISTS on existing table
CREATE TABLE t1 (id INT) ENGINE=ColumnStore;
DROP TABLE IF EXISTS t1;
SELECT * FROM t1;
ERROR 42S02: Table 'MCOL5890.t1' doesn't exist
DROP DATABASE MCOL5890;
29 changes: 29 additions & 0 deletions mysql-test/columnstore/bugfixes/mcol-5890.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
-- source ../include/have_columnstore.inc

# Setup: Create a test database
--disable_warnings
DROP DATABASE IF EXISTS MCOL5890;
CREATE DATABASE MCOL5890;
USE MCOL5890;
--enable_warnings

# Test 1: DROP TABLE IF EXISTS on a non-existent table should succeed silently
--echo # Test 1: DROP TABLE IF EXISTS on non-existent table
DROP TABLE IF EXISTS test.does_not_exist;

# Test 2: DROP TABLE on a non-existent table should fail with ER_BAD_TABLE_ERROR
--echo # Test 2: DROP TABLE on non-existent table
--error ER_BAD_TABLE_ERROR
DROP TABLE test.does_not_exist;

# Test 3: DROP TABLE IF EXISTS on an existing table should succeed
--echo # Test 3: DROP TABLE IF EXISTS on existing table
CREATE TABLE t1 (id INT) ENGINE=ColumnStore;
DROP TABLE IF EXISTS t1;

# Verify table is dropped
--error ER_NO_SUCH_TABLE
SELECT * FROM t1;

# Cleanup
DROP DATABASE MCOL5890;