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

db_sqlite.tryInsertID does raise exceptions in 1.6.0 #3

Open
gradha opened this issue Apr 24, 2022 · 2 comments
Open

db_sqlite.tryInsertID does raise exceptions in 1.6.0 #3

gradha opened this issue Apr 24, 2022 · 2 comments

Comments

@gradha
Copy link

gradha commented Apr 24, 2022

The following code compiles with nim 1.4.8:

import db_sqlite

proc mark_seen_url*(url: string) {.raises: [].} =
  var conn: DB_conn
  let result = conn.try_insert_id(sql"evil insert", url)

This code stopped compiling in 1.6.0 and later because it raises now DbError:

$ choosenim stable
   Switched to Nim 1.6.4
$ nim c dbtest.nim 
Hint: used config file '/Users/gradha/.choosenim/toolchains/nim-1.6.4/config/nim.cfg' [Conf]
Hint: used config file '/Users/gradha/.choosenim/toolchains/nim-1.6.4/config/config.nims' [Conf]
..........................................................
/Users/gradha/.choosenim/toolchains/nim-1.6.4/lib/impure/db_sqlite.nim(184, 1) Warning: Circular dependency detected. `codeReordering` pragma may not be able to reorder some nodes properly [User]
.....
/private/tmp/t/dbtest.nim(5, 7) Hint: 'result' is declared but not used [XDeclaredButNotUsed]
/private/tmp/t/dbtest.nim(3, 44) template/generic instantiation from here
/private/tmp/t/dbtest.nim(5, 20) Error: tryInsertID(conn,
  SqlQuery(r"evil insert"), [url]) can raise an unlisted exception: DbError

The odbc tryInsertId and mysql tryInsertId versions still maintain their well ment raises: [] pragma. The postgres and sqlite seem to have grown unexpected exceptions.

The sqlite module gained the API breaking change in this commit while work was being done to fix nim-lang/Nim#18669, which wasn't related to the tryInsertID proc. I haven't found any discussions in the forums or github issues about this tryInsertID API breakage. Given that other tryInsertID procs still raise no exceptions in their definitions, and a try* method ideally should keep its meaning and raise no unsuspected exceptions, this feels like a bug in both db_sqlite and db_postgres.

@ringabout
Copy link
Member

The regression of the db_sqlite is already fixed. But db_* modules is highly likely to move out of the stds. Fixing the bug of db_postgres is not high priority.

@ringabout
Copy link
Member

see nim-lang/Nim#19745 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants