Conversation
…chainlink issues, which are personal.
…d repo a real bug.
| /// `dialect` is used to *parse* the marker-bearing statement; the | ||
| /// identifier-folding policy used to *match* columns is this engine's | ||
| /// (`casing`) — pair them (e.g. `Ddx::for_duckdb()` with `DuckDbDialect`). | ||
| pub fn rewrite_sql(&self, sql: &str, dialect: &dyn Dialect) -> Result<String> { |
There was a problem hiding this comment.
Consider also an "explain" interface so folks can inspect what happens before running.
| /// expression `expr` (SQL text) with respect to the column `wrt` (a bare | ||
| /// column name), returning the derivative as SQL text — for embedding an | ||
| /// update rule where a marker can't reach (design.md §3.6). | ||
| pub fn differentiate_sql( |
There was a problem hiding this comment.
Is there an opportunity to make this more user-facing for interactive use?
| /// unsupported construct is always one of these typed errors, never an | ||
| /// approximate or silently-zero derivative. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub enum DiffError { |
There was a problem hiding this comment.
Double checking: are error messages ergonomic, stating specific guidance for users when roadbumps occur?
| ); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- |
There was a problem hiding this comment.
In the current form, are we able to retrieve multiple gradients at once using one call through SQL (instead of many individual calls)? There might be an opportunity to use a list / collection.
| fn qualified_reference_to_the_owning_cte_alias_still_errors() { | ||
| // But v.s IS v's computed alias, so differentiating it as a non-wrt term | ||
| // still crosses the projection boundary and must error. | ||
| let err = Ddx::new() |
There was a problem hiding this comment.
Seeing this made me wonder: could the project use a simple notebook or somehow interactive guidebook explaining this and other aspects?
There was a problem hiding this comment.
I like this addition! Consider also adding a Makefile or justfile which makes it super easy for developers to make these checks as code is created. This might also help with things like builds / etc.
There was a problem hiding this comment.
Consider adding a CONTRIBUTING.md file for this project also.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: dtolnay/rust-toolchain@stable |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
Consider adding a "SQL cookbook" for this project to help people quickly get up to speed on the value-add and what's possible (it's all there, we just need to make it easy to grasp within a moment!).
2 participants
WIP