search

AlaSQL / alasql

AlaSQL.js - JavaScript SQL database for browser and Node.js. Handles both traditional relational tables and nested JSON data (NoSQL). Export, store, and import data from localStorage, IndexedDB, or Excel.
http://alasql.org
MIT License
7.01k stars 653 forks source link

check for options.cache missing in exec #1164

Open umasudhan opened 4 years ago

umasudhan commented 4 years ago

I have noticed that https://github.com/agershun/alasql/blob/a725fec8943b6dce391ebff6a7258ab45205c5c3/dist/alasql.js#L4642 has a check for alasql.options.cache before it adds a statement to the cache but the line at https://github.com/agershun/alasql/blob/a725fec8943b6dce391ebff6a7258ab45205c5c3/dist/alasql.js#L11566 doesnt. Is there a reason why the check is not there in exec but is there in dexec? This is important as I am trying to reduce the RAM footprint when using ALASQL in a production environment

mathiasrw commented 4 years ago

Hi @umasudhan. Thank you for looking into this. Its very likely that we missed a detail there. Im traveling at the moment so cant dig into the details.

You are more than welcome to suggest exactly where we should add a check so we can make the library better.

umasudhan commented 4 years ago

Thanks Mathias- will submit a patch

umasudhan commented 4 years ago

Hi @mathiasrw have uploaded the patch- please advise if you would prefer a PR instead. Thanks!

check_if_cache_is_enabled_before_caching_statement.patch.txt

mathiasrw commented 4 years ago

I would really love a PR - Then the contribution is also credited correct :)

umasudhan commented 4 years ago

hi Matthias, I am unable to push to a branch I created of develop- I get a 403. Do I need some token/permission to do this? Thanks

mathiasrw commented 4 years ago

Hi @umasudhan

Have you forked the repo?

Please have a look at https://github.com/agershun/alasql/blob/develop/.github/CONTRIBUTING.md

umasudhan commented 4 years ago

HI Mathias Apologies- didnt read the readme 🤔 Will follow the process. Thanks Uma

On Sun, Mar 22, 2020 at 12:56 PM Mathias Rangel Wulff < notifications@github.com> wrote:

Hi @umasudhan https://github.com/umasudhan

Have you forked the repo?

Please have a look at https://github.com/agershun/alasql/blob/develop/.github/CONTRIBUTING.md

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/agershun/alasql/issues/1164#issuecomment-602135374, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADZJ7LXAERQCKTSO32P7TALRIVV45ANCNFSM4KTS3HMA .

mathiasrw commented 2 years ago

Do you still have that repo?