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

Native support for BigInt data type for SUM() and other aggregate functions #1977

Closed
HamudeHomsi opened this issue Sep 30, 2024 · 8 comments
Closed

Comments

@HamudeHomsi
Copy link

HamudeHomsi commented Sep 30, 2024

Hello, SUM() and other aggr functions should also support BigInt data type beside Number.
BigInt is part of JavaScript (since ES2020) and is increasingly important for handling large integers without loss of precision.

  • Describe the problem.
    If the column in the table is stored as bigint then SUM() function should add them together.
    Possible solution: check the typeof of the first value and based on this, sum all values and return the result as the given type.
    Workaround: manually create an aggregate function:
import alasql from "alasql";

alasql.fn.SUM64 = function(value, accumulator, stage) {
	if (stage == 1) {
		var newAccumulator = value;
		return newAccumulator;
	} else if (stage == 2) {
		accumulator = accumulator + value;
		return accumulator;
	} else if (stage == 3) {
		return accumulator;
	}
};
  • Provide data and code that replicates the problem.
    "node": "^18.x"
    "alasql": "^4.5.1"
import alasql from "alasql";

this.database = new alasql.Database();
this.database.exec("CREATE TABLE myTable (id INT PRIMARY KEY AUTOINCREMENT, name STRING, size BIGINT)");
this.database.exec("INSERT INTO myTable (name, size) VALUES (?, ?), ['first item name', 10n];
this.database.exec("INSERT INTO myTable (name, size) VALUES (?, ?), ['second item name', 214748364733443n];

Where this.database.exec("SELECT SUM(size) FROM myTable") => Error because it cannot add a bigint data type with a number data type.

@mathiasrw
Copy link
Member

mathiasrw commented Oct 1, 2024

Great idea!

I replicated the problem in https://jsfiddle.net/yb4fteas/

It seems to be the sanitiser that is blocking for the use of BigInt

@KamilStefanco
Copy link
Contributor

Hello, i would like to work on this issue.

@mathiasrw
Copy link
Member

We are sanetising by converting to JSON and back. A solid regex that only runs on string would be much more efficient and solve this problem.

@KamilStefanco
Copy link
Contributor

Thank you for assigning the issue. I made a test for this issue as instructed. Any tips on where and how to fix this?

@mathiasrw
Copy link
Member

hm. Not super sure where it is. You can try the old fashion and insert console.log close to all the places we call JSON.stringify - or you can try to recreate the problem using the code from my previous JSfiddle as that will throw the actual error providing you with a line 

KamilStefanco pushed a commit to KamilStefanco/alasql that referenced this issue Oct 14, 2024
@Charlesnorris509
Copy link

hey @mathiasrw @KamilStefanco Did this issue got resolved if not I would like to participate finding the solution, I would love to be assigned with this issue

@KamilStefanco
Copy link
Contributor

I already submitted a pull request for this issue in #1981

@mathiasrw
Copy link
Member

Released as part of v4.6.0

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

No branches or pull requests

4 participants