From 0ee5bf6fb1e16c2d09c829024719f224803d3ba2 Mon Sep 17 00:00:00 2001 From: Tmonster Date: Wed, 26 Feb 2025 11:24:40 +0100 Subject: [PATCH 1/3] remove .open --- tools/shell/shell.cpp | 66 +------------------------- tools/shell/tests/test_safe_mode.py | 4 +- tools/shell/tests/test_shell_basics.py | 26 ---------- 3 files changed, 3 insertions(+), 93 deletions(-) diff --git a/tools/shell/shell.cpp b/tools/shell/shell.cpp index 60aa2368105e..7db5c35d6f3e 100644 --- a/tools/shell/shell.cpp +++ b/tools/shell/shell.cpp @@ -2236,11 +2236,6 @@ static const char *azHelp[] = { " --bom Put a UTF8 byte-order mark at the beginning", " -e Send output to the system text editor", " -x Send output as CSV to a spreadsheet (same as \".excel\")", - ".open ?OPTIONS? ?FILE? Close existing database and reopen FILE", - " Options:", - " --new Initialize FILE to an empty database", - " --nofollow Do not follow symbolic links", - " --readonly Open FILE readonly", ".output ?FILE? Send output to FILE or stdout if FILE is omitted", " If FILE begins with '|' then open it as a pipe.", " Options:", @@ -3619,64 +3614,6 @@ MetadataResult ImportData(ShellState &state, const char **azArg, idx_t nArg) { return MetadataResult::SUCCESS; } -bool ShellState::OpenDatabase(const char **azArg, idx_t nArg) { - if (safe_mode) { - utf8_printf(stderr, ".open cannot be used in -safe mode\n"); - return false; - } - char *zNewFilename; /* Name of the database file to open */ - idx_t iName = 1; /* Index in azArg[] of the filename */ - bool newFlag = false; /* True to delete file before opening */ - /* Close the existing database */ - close_db(db); - db = nullptr; - globalDb = nullptr; - zDbFilename = string(); - openMode = SHELL_OPEN_UNSPEC; - openFlags = openFlags & ~(SQLITE_OPEN_NOFOLLOW); // don't overwrite settings loaded in the command line - szMax = 0; - /* Check for command-line arguments */ - for (idx_t iName = 1; iName < nArg && azArg[iName][0] == '-'; iName++) { - const char *z = azArg[iName]; - if (optionMatch(z, "new")) { - newFlag = true; - } else if (optionMatch(z, "readonly")) { - openMode = SHELL_OPEN_READONLY; - } else if (optionMatch(z, "nofollow")) { - openFlags |= SQLITE_OPEN_NOFOLLOW; - } else if (z[0] == '-') { - utf8_printf(stderr, "unknown option: %s\n", z); - return false; - } - } - /* If a filename is specified, try to open it first */ - zNewFilename = nArg > iName ? sqlite3_mprintf("%s", azArg[iName]) : 0; - if (zNewFilename || openMode == SHELL_OPEN_HEXDB) { - if (newFlag) { - shellDeleteFile(zNewFilename); - } - zDbFilename = zNewFilename; - sqlite3_free(zNewFilename); - OpenDB(OPEN_DB_KEEPALIVE); - if (!db) { - utf8_printf(stderr, "Error: cannot open '%s'\n", zNewFilename); - } - } - if (!db) { - /* As a fall-back open a TEMP database */ - zDbFilename = string(); - OpenDB(0); - } - return true; -} - -MetadataResult OpenDatabase(ShellState &state, const char **azArg, idx_t nArg) { - if (!state.OpenDatabase(azArg, nArg)) { - return MetadataResult::FAIL; - } - return MetadataResult::SUCCESS; -} - MetadataResult PrintArguments(ShellState &state, const char **azArg, idx_t nArg) { for (idx_t i = 1; i < nArg; i++) { if (i > 1) { @@ -4196,7 +4133,6 @@ static const MetadataCommand metadata_commands[] = { {"mode", 0, SetOutputMode, "MODE ?TABLE?", "Set output mode", 0}, {"nullvalue", 2, SetNullValue, "STRING", "Use STRING in place of NULL values", 0}, - {"open", 0, OpenDatabase, "?OPTIONS? ?FILE?", "Close existing database and reopen FILE", 2}, {"once", 0, SetOutputOnce, "?FILE?", "Output for the next SQL command only to FILE", 0}, {"output", 0, SetOutput, "?FILE?", "Send output to FILE or stdout if FILE is omitted", 0}, {"print", 0, PrintArguments, "STRING...", "Print literal STRING", 3}, @@ -5110,7 +5046,7 @@ int SQLITE_CDECL wmain(int argc, wchar_t **wargv) { ShellHighlight highlighter(data); highlighter.PrintText("transient in-memory database", PrintOutput::STDOUT, PrintColor::STANDARD, PrintIntensity::BOLD); - printf(".\nUse \".open FILENAME\" to reopen on a " + printf(".\nCall \"Attach 'FILENAME' AS database_name; USE database_name;\" to reopen on a " "persistent database.\n"); } zHistory = getenv("DUCKDB_HISTORY"); diff --git a/tools/shell/tests/test_safe_mode.py b/tools/shell/tests/test_safe_mode.py index 258075a945bf..42efc3a36071 100644 --- a/tools/shell/tests/test_safe_mode.py +++ b/tools/shell/tests/test_safe_mode.py @@ -8,7 +8,7 @@ from tools.shell.tests.conftest import random_filepath -@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".open new_file", ".output out", ".once out", ".excel out", ".read myfile.sql"]) +@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".output out", ".once out", ".excel out", ".read myfile.sql"]) def test_safe_mode_command(shell, command): test = ( ShellTest(shell, ['-safe']) @@ -40,7 +40,7 @@ def test_safe_mode_database_basic(shell, random_filepath): result = test.run() result.check_stdout("6") -@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".open new_file", ".output out", ".once out", ".excel out", ".read myfile.sql"]) +@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".output out", ".once out", ".excel out", ".read myfile.sql"]) @pytest.mark.parametrize("persistent", [False, True]) def test_safe_mode_database_commands(shell, random_filepath, command, persistent): arguments = ['-safe'] if not persistent else [random_filepath, '-safe'] diff --git a/tools/shell/tests/test_shell_basics.py b/tools/shell/tests/test_shell_basics.py index e2c554bb5a2d..8c29a33556ee 100644 --- a/tools/shell/tests/test_shell_basics.py +++ b/tools/shell/tests/test_shell_basics.py @@ -673,32 +673,6 @@ def test_mode_tabs(shell): result = test.run() result.check_stdout('fourty-two') -def test_open(shell, tmp_path): - file_one = tmp_path / "file_one" - file_two = tmp_path / "file_two" - test = ( - ShellTest(shell) - .statement(f".open {file_one.as_posix()}") - .statement("CREATE TABLE t1 (i INTEGER);") - .statement("INSERT INTO t1 VALUES (42);") - .statement(f".open {file_two.as_posix()}") - .statement("CREATE TABLE t2 (i INTEGER);") - .statement("INSERT INTO t2 VALUES (43);") - .statement(f".open {file_one.as_posix()}") - .statement("SELECT * FROM t1;") - ) - result = test.run() - result.check_stdout('42') - -@pytest.mark.parametrize('generated_file', ["blablabla"], indirect=True) -def test_open_non_database(shell, generated_file): - test = ( - ShellTest(shell) - .add_argument(generated_file.as_posix()) - ) - result = test.run() - result.check_stderr('not a valid DuckDB database file') - def test_enable_profiling(shell): test = ( ShellTest(shell) From ae47e8c6b244dafc5ad1079d6afd58b8801e38c8 Mon Sep 17 00:00:00 2001 From: Tmonster Date: Wed, 26 Feb 2025 12:28:10 +0100 Subject: [PATCH 2/3] Revert "remove .open" This reverts commit 0ee5bf6fb1e16c2d09c829024719f224803d3ba2. --- tools/shell/shell.cpp | 66 +++++++++++++++++++++++++- tools/shell/tests/test_safe_mode.py | 4 +- tools/shell/tests/test_shell_basics.py | 26 ++++++++++ 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/tools/shell/shell.cpp b/tools/shell/shell.cpp index 7db5c35d6f3e..60aa2368105e 100644 --- a/tools/shell/shell.cpp +++ b/tools/shell/shell.cpp @@ -2236,6 +2236,11 @@ static const char *azHelp[] = { " --bom Put a UTF8 byte-order mark at the beginning", " -e Send output to the system text editor", " -x Send output as CSV to a spreadsheet (same as \".excel\")", + ".open ?OPTIONS? ?FILE? Close existing database and reopen FILE", + " Options:", + " --new Initialize FILE to an empty database", + " --nofollow Do not follow symbolic links", + " --readonly Open FILE readonly", ".output ?FILE? Send output to FILE or stdout if FILE is omitted", " If FILE begins with '|' then open it as a pipe.", " Options:", @@ -3614,6 +3619,64 @@ MetadataResult ImportData(ShellState &state, const char **azArg, idx_t nArg) { return MetadataResult::SUCCESS; } +bool ShellState::OpenDatabase(const char **azArg, idx_t nArg) { + if (safe_mode) { + utf8_printf(stderr, ".open cannot be used in -safe mode\n"); + return false; + } + char *zNewFilename; /* Name of the database file to open */ + idx_t iName = 1; /* Index in azArg[] of the filename */ + bool newFlag = false; /* True to delete file before opening */ + /* Close the existing database */ + close_db(db); + db = nullptr; + globalDb = nullptr; + zDbFilename = string(); + openMode = SHELL_OPEN_UNSPEC; + openFlags = openFlags & ~(SQLITE_OPEN_NOFOLLOW); // don't overwrite settings loaded in the command line + szMax = 0; + /* Check for command-line arguments */ + for (idx_t iName = 1; iName < nArg && azArg[iName][0] == '-'; iName++) { + const char *z = azArg[iName]; + if (optionMatch(z, "new")) { + newFlag = true; + } else if (optionMatch(z, "readonly")) { + openMode = SHELL_OPEN_READONLY; + } else if (optionMatch(z, "nofollow")) { + openFlags |= SQLITE_OPEN_NOFOLLOW; + } else if (z[0] == '-') { + utf8_printf(stderr, "unknown option: %s\n", z); + return false; + } + } + /* If a filename is specified, try to open it first */ + zNewFilename = nArg > iName ? sqlite3_mprintf("%s", azArg[iName]) : 0; + if (zNewFilename || openMode == SHELL_OPEN_HEXDB) { + if (newFlag) { + shellDeleteFile(zNewFilename); + } + zDbFilename = zNewFilename; + sqlite3_free(zNewFilename); + OpenDB(OPEN_DB_KEEPALIVE); + if (!db) { + utf8_printf(stderr, "Error: cannot open '%s'\n", zNewFilename); + } + } + if (!db) { + /* As a fall-back open a TEMP database */ + zDbFilename = string(); + OpenDB(0); + } + return true; +} + +MetadataResult OpenDatabase(ShellState &state, const char **azArg, idx_t nArg) { + if (!state.OpenDatabase(azArg, nArg)) { + return MetadataResult::FAIL; + } + return MetadataResult::SUCCESS; +} + MetadataResult PrintArguments(ShellState &state, const char **azArg, idx_t nArg) { for (idx_t i = 1; i < nArg; i++) { if (i > 1) { @@ -4133,6 +4196,7 @@ static const MetadataCommand metadata_commands[] = { {"mode", 0, SetOutputMode, "MODE ?TABLE?", "Set output mode", 0}, {"nullvalue", 2, SetNullValue, "STRING", "Use STRING in place of NULL values", 0}, + {"open", 0, OpenDatabase, "?OPTIONS? ?FILE?", "Close existing database and reopen FILE", 2}, {"once", 0, SetOutputOnce, "?FILE?", "Output for the next SQL command only to FILE", 0}, {"output", 0, SetOutput, "?FILE?", "Send output to FILE or stdout if FILE is omitted", 0}, {"print", 0, PrintArguments, "STRING...", "Print literal STRING", 3}, @@ -5046,7 +5110,7 @@ int SQLITE_CDECL wmain(int argc, wchar_t **wargv) { ShellHighlight highlighter(data); highlighter.PrintText("transient in-memory database", PrintOutput::STDOUT, PrintColor::STANDARD, PrintIntensity::BOLD); - printf(".\nCall \"Attach 'FILENAME' AS database_name; USE database_name;\" to reopen on a " + printf(".\nUse \".open FILENAME\" to reopen on a " "persistent database.\n"); } zHistory = getenv("DUCKDB_HISTORY"); diff --git a/tools/shell/tests/test_safe_mode.py b/tools/shell/tests/test_safe_mode.py index 42efc3a36071..258075a945bf 100644 --- a/tools/shell/tests/test_safe_mode.py +++ b/tools/shell/tests/test_safe_mode.py @@ -8,7 +8,7 @@ from tools.shell.tests.conftest import random_filepath -@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".output out", ".once out", ".excel out", ".read myfile.sql"]) +@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".open new_file", ".output out", ".once out", ".excel out", ".read myfile.sql"]) def test_safe_mode_command(shell, command): test = ( ShellTest(shell, ['-safe']) @@ -40,7 +40,7 @@ def test_safe_mode_database_basic(shell, random_filepath): result = test.run() result.check_stdout("6") -@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".output out", ".once out", ".excel out", ".read myfile.sql"]) +@pytest.mark.parametrize("command", [".sh ls", ".cd ..", ".log file", ".import file.csv tbl", ".open new_file", ".output out", ".once out", ".excel out", ".read myfile.sql"]) @pytest.mark.parametrize("persistent", [False, True]) def test_safe_mode_database_commands(shell, random_filepath, command, persistent): arguments = ['-safe'] if not persistent else [random_filepath, '-safe'] diff --git a/tools/shell/tests/test_shell_basics.py b/tools/shell/tests/test_shell_basics.py index 8c29a33556ee..e2c554bb5a2d 100644 --- a/tools/shell/tests/test_shell_basics.py +++ b/tools/shell/tests/test_shell_basics.py @@ -673,6 +673,32 @@ def test_mode_tabs(shell): result = test.run() result.check_stdout('fourty-two') +def test_open(shell, tmp_path): + file_one = tmp_path / "file_one" + file_two = tmp_path / "file_two" + test = ( + ShellTest(shell) + .statement(f".open {file_one.as_posix()}") + .statement("CREATE TABLE t1 (i INTEGER);") + .statement("INSERT INTO t1 VALUES (42);") + .statement(f".open {file_two.as_posix()}") + .statement("CREATE TABLE t2 (i INTEGER);") + .statement("INSERT INTO t2 VALUES (43);") + .statement(f".open {file_one.as_posix()}") + .statement("SELECT * FROM t1;") + ) + result = test.run() + result.check_stdout('42') + +@pytest.mark.parametrize('generated_file', ["blablabla"], indirect=True) +def test_open_non_database(shell, generated_file): + test = ( + ShellTest(shell) + .add_argument(generated_file.as_posix()) + ) + result = test.run() + result.check_stderr('not a valid DuckDB database file') + def test_enable_profiling(shell): test = ( ShellTest(shell) From f382feb8e4cc80ff2ace647601cf2b0b5a7c08f8 Mon Sep 17 00:00:00 2001 From: Tmonster Date: Mon, 3 Mar 2025 09:57:41 +0100 Subject: [PATCH 3/3] attempt to call attach from within .open --- tools/shell/shell.cpp | 76 ++++++++++--------- .../tests/test_backwards_compatibility.py | 12 +-- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/tools/shell/shell.cpp b/tools/shell/shell.cpp index 60aa2368105e..31df60f4fdb2 100644 --- a/tools/shell/shell.cpp +++ b/tools/shell/shell.cpp @@ -3626,48 +3626,54 @@ bool ShellState::OpenDatabase(const char **azArg, idx_t nArg) { } char *zNewFilename; /* Name of the database file to open */ idx_t iName = 1; /* Index in azArg[] of the filename */ - bool newFlag = false; /* True to delete file before opening */ - /* Close the existing database */ - close_db(db); - db = nullptr; - globalDb = nullptr; zDbFilename = string(); - openMode = SHELL_OPEN_UNSPEC; - openFlags = openFlags & ~(SQLITE_OPEN_NOFOLLOW); // don't overwrite settings loaded in the command line - szMax = 0; - /* Check for command-line arguments */ - for (idx_t iName = 1; iName < nArg && azArg[iName][0] == '-'; iName++) { - const char *z = azArg[iName]; - if (optionMatch(z, "new")) { - newFlag = true; - } else if (optionMatch(z, "readonly")) { - openMode = SHELL_OPEN_READONLY; - } else if (optionMatch(z, "nofollow")) { - openFlags |= SQLITE_OPEN_NOFOLLOW; - } else if (z[0] == '-') { - utf8_printf(stderr, "unknown option: %s\n", z); - return false; - } + bool read_only = false; + // 2 is the first non filename arg + for (idx_t iName = 2; iName < nArg && azArg[iName][0] == '-'; iName++) { + const char *z = azArg[iName]; + if (optionMatch(z, "readonly")) { + read_only = true; + } else if (z[0] == '-') { + utf8_printf(stderr, "unknown option: %s\n", z); + return false; + } } /* If a filename is specified, try to open it first */ zNewFilename = nArg > iName ? sqlite3_mprintf("%s", azArg[iName]) : 0; - if (zNewFilename || openMode == SHELL_OPEN_HEXDB) { - if (newFlag) { - shellDeleteFile(zNewFilename); - } + if (zNewFilename) { zDbFilename = zNewFilename; sqlite3_free(zNewFilename); - OpenDB(OPEN_DB_KEEPALIVE); - if (!db) { - utf8_printf(stderr, "Error: cannot open '%s'\n", zNewFilename); - } - } - if (!db) { - /* As a fall-back open a TEMP database */ - zDbFilename = string(); - OpenDB(0); + idx_t dot_placement = zDbFilename.find("."); + bool found_extension = dot_placement >= 0; + string zDbDBName; + if (!found_extension) { + zDbDBName = zDbFilename; + } else { + zDbDBName = zDbFilename.substr(0, dot_placement); + } + string attach = "ATTACH '" + zDbFilename + "' as " + zDbDBName; + if (read_only) { + attach += " (READ_ONLY)"; + } + attach += ";"; + const char *attach_sql = attach.c_str(); + string use = " USE " + zDbDBName + ";"; + const char *use_sql = use.c_str(); + char *zErrMsg = 0; + sqlite3_exec(db, attach_sql, NULL, NULL, &zErrMsg); + if (zErrMsg) { + utf8_printf(stderr, "%s\n", zErrMsg); + return false; + }; + sqlite3_exec(db, use_sql, NULL, NULL, &zErrMsg); + if (zErrMsg) { + utf8_printf(stderr, "%s\n", zErrMsg); + return false; + }; + return true; } - return true; + utf8_printf(stderr, "Valid Filename not provided\n"); + return false; } MetadataResult OpenDatabase(ShellState &state, const char **azArg, idx_t nArg) { diff --git a/tools/shell/tests/test_backwards_compatibility.py b/tools/shell/tests/test_backwards_compatibility.py index 98d5135d1790..c36f196ed021 100644 --- a/tools/shell/tests/test_backwards_compatibility.py +++ b/tools/shell/tests/test_backwards_compatibility.py @@ -10,7 +10,7 @@ def test_version_dev(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_dev.db") + .statement("Attach 'test/storage/bc/db_dev.db' as db_dev;") ) result = test.run() result.check_stderr("older development version") @@ -18,7 +18,7 @@ def test_version_dev(shell): def test_version_0_3_1(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_031.db") + .statement("Attach 'test/storage/bc/db_031.db' as db_031;") ) result = test.run() result.check_stderr("v0.3.1") @@ -26,7 +26,7 @@ def test_version_0_3_1(shell): def test_version_0_3_2(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_032.db") + .statement("Attach 'test/storage/bc/db_032.db' as db_032;") ) result = test.run() result.check_stderr("v0.3.2") @@ -34,7 +34,7 @@ def test_version_0_3_2(shell): def test_version_0_4(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_04.db") + .statement("Attach 'test/storage/bc/db_04.db' as db_04;") ) result = test.run() result.check_stderr("v0.4.0") @@ -42,7 +42,7 @@ def test_version_0_4(shell): def test_version_0_5_1(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_051.db") + .statement("Attach 'test/storage/bc/db_051.db' as db_051;") ) result = test.run() result.check_stderr("v0.5.1") @@ -50,7 +50,7 @@ def test_version_0_5_1(shell): def test_version_0_6_0(shell): test = ( ShellTest(shell) - .statement(".open test/storage/bc/db_060.db") + .statement("Attach 'test/storage/bc/db_060.db' as db_060;") ) result = test.run() result.check_stderr("v0.6.0")