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

Enable new math functions from 3.35.0 #763

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

gotson
Copy link
Collaborator

@gotson gotson commented Aug 12, 2022

This will address #633.

As mentioned in the issue, existing math functions are currently supported by user contributed code (extension-functions.c from https://sqlite.org/contrib).

The https://www.sqlite.org/draft/lang_mathfunc.html introduced in 3.35.0 overlap with some of the previously existing functions.

Since the new functions can be enabled with SQLITE_ENABLE_MATH_FUNCTIONS, the old functions will be included only if the new math functions are not enabled. The other functions from that old contrib files would still be included.

⚠️ It is worth noting that there is a breaking change with the log(X) function:

  • currently it performs ln(X)
  • with the new math functions it performs log10(X)

Here is an explanation from the SQLite website:

Compatibility note: SQLite works like PostgreSQL in that the log() function computes a base-10 logarithm. Most other SQL database engines compute a natural logarithm for log(). In the two-argument version of log(B,X), the first argument is the base and the second argument is the operand. This is the same as in PostgreSQL and MySQL, but is reversed from SQL Server which uses the second argument as the base and the first argument as the operand.

Given our versioning scheme, I think we should only introduce breaking changes when updating to a major version of SQLite, as users would most likely perform more regression tests on major version updates.

This PR will sit still until we can upgrade to the future 3.40.0 version of SQLite.

@gotson gotson added enhancement:SQLite Enhancement about sqlite features breaking change Change that will impact existing behaviour labels Aug 12, 2022
@gotson gotson marked this pull request as draft August 12, 2022 03:18
@gotson gotson linked an issue Aug 12, 2022 that may be closed by this pull request
steventamm added a commit to salesforce/formula-engine that referenced this pull request Oct 23, 2022
Using ROUND(...,33) in sqlite causes more issues than it solves for exact to inexact conversion
  for ceil & floor.  So allow the SqlHooks to turn off the rounding and control the precision
Have special hooks for testing without the standard math functions, since the sqlite-jdbc
  doesn't use the standard math functions (see xerial/sqlite-jdbc#763)
Turn off binding because the default driver uses numbers for date binding when testing
More fixes for sqlite for date math
steventamm added a commit to salesforce/formula-engine that referenced this pull request Oct 24, 2022
* Sqlite 3 Support

Sqlite3 is the default database in iOS, and generating custom formulas offline in javascript is
  great, but being able to query the offline cache directly is even better.

General Changes
- DatePart extraction moved to hooks to make it easier to reason with
- Log & NaturalLog moved to hooks as well, since Sqlite doesn't have a standard implementation

* Sqlite precision/driver issues

Using ROUND(...,33) in sqlite causes more issues than it solves for exact to inexact conversion
  for ceil & floor.  So allow the SqlHooks to turn off the rounding and control the precision
Have special hooks for testing without the standard math functions, since the sqlite-jdbc
  doesn't use the standard math functions (see xerial/sqlite-jdbc#763)
Turn off binding because the default driver uses numbers for date binding when testing
More fixes for sqlite for date math

* Final fixes to get Sqlite3 to work

Add in goldfiles
Duration, addMonths now work
Simulate lpad/rpad using zeroblob trick
FormatCurrency, IsoWeek/Month, InitCap still don't work

* Enable sqlite-test by when running db-tests

Fix goldfile difference in spanner due to error message change due to fallback on date/tstamp
  retrievable from the jdbc resultset
steventamm added a commit to salesforce/formula-engine that referenced this pull request Oct 25, 2022
* Version 0.3.0
* Better support for multiple DBs
  - Allow nested whyIgnoreSql elements in formulatests.xml instead of an attribute to make
    merges, readability easier
  - Allow specifying whether to ignore SQL because it's unimplemented or a specific number
    of failure so that various sql issues aren't just swept under the rug.
  - Be more precise about why SQL is failures and split formulatext.xml
  - Do some refactoring around date/time logic to rely on the hooks for more
  - Move math-like formulas to formulatests-math.xml
  - Use Concat and not + for JsonValue
  - DatePart extraction moved to hooks to make it easier to reason with
  - Log & NaturalLog moved to hooks as well, since Sqlite doesn't have a standard implementation
* Add in support for Google Standard SQL
  - Run tests against a Spanner local emulator
  - Google Standard SQL has distinct date and timestamp, so some functions/hooks
    needed to be split up between date and datetime since
    google standard sql doesn't autoconvert date to datetime
  - Google has no time type at all, so no reliance on any TIME at all
  - Spanner has a maximum scale of 9 and lower precision than other DBs, so
    the binding of BigDecimals needs to be customized to be only 9
  - Implement numFailures for spanner.
  - Fix goldfile difference in spanner due to error message change due to fallback on date/tstamp
  retrievable from the jdbc resultset
* Mysql/MarisDB fixes
- Abstract Mysql and MariaDB testers to share time parsing stuff.
- MariaDB's TIME function doesn't parse correctly, so have a separate hook for that.
- MariaDB's JDBC driver uses time, so only convert in MySQL to integer
  Note: MariaDB tests are not run by default for perf reasons

* Presto Support (for Amazon Athena and the like)
  - Support for Presto/Trino with the formula engine to support Presto-style DBs
  such as Amazon Athena.  It uses TrinoDB to test with testcontainers, but
  is restricted to Presto 0.277 syntax for backwards compatibility
  - Note: These tests aren't run with -P db-tests.  It's just too slow.  
   - But, like MariaDB, it works if you enable it or run it by hand.
* Presto Changes
- Query exceptions in Trino contain a UUID, so that has to be stripped with
   getSqlExceptionMessage()
- Presto doesn't support unary +, so distance needed some work to remove it
- Presto is more fiddly with timestamp vs date than other DBs
- Date math more fully implemented
- Rounding mode is HALF_UP, not HALF_EVEN unlike other DBs.  Causes some issues.
- Make superclass of Presto and TrinoContainerTester
- Presto with ' / 100.0' for percent was reducing the scale and making some
  of the numbers wrong.  Since you can't use binding, and have to use
  literal because of the JDBC BD bind issue, so this improves things
- Use explicit decimal literals with Presto instead of 100.0 to prevent truncation
- Log/Ln need explicit decimal casting.  Others may as well..
- Make presto TRUNC return DECIMAL(38,18)
- Use more decimals with the percent conversion and the conversion it in the SqlHooks.
- Presto can't format intervals, so don't bother with them.  Do it using the java
  format stuff that's in Presto/Trino
- Use trim when parsing dates to match java
- Use DECIMAL '100.' to prevent decimal overflow when using percents.
  There's still precision differences in functions with it, however
- Ignore precision differences like in postgres
- Time formatting fixes
- Ignore some functions that aren't supported like "right"

* Sqlite3 Support (#91)
- Sqlite3 is the default database in iOS, and generating custom formulas offline in javascript is
  great, but being able to query the offline cache directly is even better.
- Using ROUND(...,33) in sqlite causes more issues than it solves for exact to inexact conversion
  for ceil & floor.  So allow the SqlHooks to turn off the rounding and control the precision
- Have special hooks for testing without the standard math functions, since the sqlite-jdbc
  doesn't use the standard math functions (see xerial/sqlite-jdbc#763)
- Turn off binding because the default driver uses numbers for date binding when testing
- Simulate lpad/rpad using zeroblob trick
- FormatCurrency, IsoWeek/Month, InitCap still don't work
- Mac and Linux are... different when it comes to their precision and implementations of certain
  mathematical functions.  So many of the tests have different results for Mac vs Linux which makes
  CI/CD difficult.  Paper over these difficulties by rounding the output to 12 decimals at most when
  comparing the results.
- Remove test with Mac vs Linux differences from Sqlite testing
  Linux throws an "Numerical result out of range" exception that Mac doesn't
  Since we don't want different results for different architectures, paper over the difference by
  returning null when doing an overflow, which the Mac version of sqlite doesIt's too variable to manage, 
  so just ignore the tests: testExponentiationOperator, testTruncUsesIf, and testTruncUsesTruncMinus
  Will try again with 3.40
* Coverage Improvements
- Coverage for composite command and getFieldPathIdDirect...
- Test inaccessible field strategy
- And remove warnings for duplicate versions.
@gotson gotson marked this pull request as ready for review November 21, 2022 06:28
@gotson
Copy link
Collaborator Author

gotson commented Nov 21, 2022

/native

@gotson
Copy link
Collaborator Author

gotson commented Nov 21, 2022

/native

use the new math functions introduced in sqlite 3.35.0
this replaces some of the existing functions

BREAKING-CHANGE: previously log() computed the natural logarithm, now it computes a base-10 logarithm
@gotson gotson merged commit 0f41f46 into xerial:master Nov 21, 2022
@gotson gotson deleted the feat/gh-633/math-functions branch November 21, 2022 09:59
@thipages
Copy link

As a note, I was using an old 3.39 version (with DBeaver) and got some troubles getting ln from log.
Now everything is fine. Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Change that will impact existing behaviour enhancement:SQLite Enhancement about sqlite features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some of new math function are missing
2 participants