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

Main future branch #9

Closed

Conversation

belamenso
Copy link

No description provided.

belamenso and others added 5 commits November 2, 2024 16:31
right now on `sbt run test` it fails due to a misplaced file:
[error] java.sql.SQLException: java.sql.SQLException: IO Error: No files found that match the pattern "/home/bbi/ticklish/tyql/bench/data/tc/edge.csv"
[error] 	at org.duckdb.DuckDBPreparedStatement.prepare(DuckDBPreparedStatement.java:121)
[error] 	at org.duckdb.DuckDBPreparedStatement.execute(DuckDBPreparedStatement.java:195)
[error] 	at tyql.main$package$.main(main.scala:57)
[error] 	at tyql.main.main(main.scala:39)
[error] 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
[error] 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
[error] Caused by: java.sql.SQLException: IO Error: No files found that match the pattern "/home/bbi/ticklish/tyql/bench/data/tc/edge.csv"
[error] 	at org.duckdb.DuckDBNative.duckdb_jdbc_prepare(Native Method)
[error] 	at org.duckdb.DuckDBPreparedStatement.prepare(DuckDBPreparedStatement.java:115)
[error] 	at org.duckdb.DuckDBPreparedStatement.execute(DuckDBPreparedStatement.java:195)
[error] 	at tyql.main$package$.main(main.scala:57)
[error] 	at tyql.main.main(main.scala:39)
[error] 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
[error] 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
[error] stack trace is suppressed; run last Compile / run for the full output
[error] (Compile / run) java.sql.SQLException: java.sql.SQLException: IO Error: No files found that match the pattern "/home/bbi/ticklish/tyql/bench/data/tc/edge.csv"
[error] Total time: 1 s, completed 2 Nov 2024, 16:30:15
@belamenso
Copy link
Author

I've copied over basic test to see if we have a connection.
Adding the dependencies on the DB drivers did significantly increase the time it takes to sbt update from a clean state.

@belamenso belamenso changed the base branch from main to tyql-improvements November 14, 2024 22:30
There is only one tag now, for expensive tests. Untagged tests are cheap.
`sbt test` runs all tests.
`sbt "testOnly -- --include-tags=Expensive"` runs only the expensive tests.
`sbt "testOnly -- --exclude-tags=Expensive"` runs only the cheap tests.
@belamenso belamenso force-pushed the better-containerization branch from 523aec1 to 2a2bbbb Compare November 15, 2024 00:25
@belamenso
Copy link
Author

@aherlihy Why did you decide on this specific munit version?
"org.scalameta" %% "munit" % "1.0.0+24-ee555b1d-SNAPSHOT" % Test,

@belamenso belamenso changed the title Better containerization Better containerization and some other changes Nov 18, 2024
@belamenso
Copy link
Author

belamenso commented Nov 18, 2024

@aherlihy This is the compile-time error message that happens when a user wants to use a dialect feature that was not defined for the current dialect:

-- [E172] Type Error: /app/src/test/scala/test/integration/random.scala:60:39 --
60 |      check(t.map(_ => Expr.randomFloat()).toQueryIR.toSQLString())(withDBNoImplicits.sqlite)
   |                                       ^
   |No given instance of type tyql.DialectFeature.RandomFloat was found for parameter r of method randomFloat in object Expr
   |
   |One of the following imports might fix the problem:
   |
   |  import tyql.Dialect.duckdb.given_RandomFloat
   |  import tyql.Dialect.h2.given_RandomFloat
   |  import tyql.Dialect.mariadb.given_RandomFloat
   |  import tyql.Dialect.mysql.given_RandomFloat
   |  import tyql.Dialect.postgresql.given_RandomFloat
   |

What do you think of the readability of this? We could define it as something that fails at runtime, but then the compiler will not error and the IDE will not draw red lines under incorrect code. What do you think?

It tell you which dialects have it, but it a shame that it does not show you somehow the current dialect (but that's in the source code unless you're using ANSI SQL which is the default).

@belamenso
Copy link
Author

On my laptop I sometimes get this error when running tests

java.sql.SQLException: No suitable driver found for jdbc:postgresql://localhost:5433/testdb
    at java.sql.DriverManager.getConnection(DriverManager.java:708)
    at java.sql.DriverManager.getConnection(DriverManager.java:230)

but this happens only on my laptop, it works perfectly on Docker. If you see this, it's not a problem, I think.

@belamenso
Copy link
Author

belamenso commented Nov 25, 2024

GitHub CI is already enabled on this branch: https://github.com/belamenso/tyql/tree/better-containerization
image

These tests run for around ~3 minutes (mostly installing). This could perhaps be optimized, but I don't know Docker/docker-compose well enough for now.

@belamenso
Copy link
Author

belamenso commented Nov 26, 2024

@aherlihy Questions is, what to do with equality and null-safe equality. Postgres and DuckDB and H2 will fail at runtime when you try to compare things that are not of the same type affinity (!). Our options

  • make equalities only between the same type affinities
  • make equalities universal and fail at runtime in these special DBs
  • make equalities universal except for these two dialects, where they are only between the same type affinities
  • polyfill these special DBs somehow, ideally only for questionable type combinations

Example things that fail at runtime in Postgres:

  • select 'b' is not distinct from 10
  • select 'a' = 10

For now what you can compare is fully dialect-dependent.

@belamenso belamenso changed the title Better containerization and some other changes Main future branch Dec 1, 2024
@belamenso
Copy link
Author

The main branch of my fork is now on main, I close this pull-request and will open another.

@belamenso belamenso closed this Dec 21, 2024
@belamenso belamenso mentioned this pull request Dec 21, 2024
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

Successfully merging this pull request may close these issues.

2 participants