patrickvorgers / Home-Assistant-Import-Energy-Data

Import historical energy data into Home Assistant so that it can be used in the Energy Dashboard
MIT License
74 stars 7 forks source link

SQLite Script syntax error #33

Closed sudomax closed 3 months ago

sudomax commented 3 months ago

Error when executing the SQL script:

L'exécution s'est terminée avec des erreurs.
Résultat : near "UPDATE": syntax error
À la ligne 314 :
INSERT INTO statistics (state, sum, metadata_id, created_ts, start_ts)
SELECT new_sum, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true
ON CONFLICT DO UPDATE
patrickvorgers commented 3 months ago

I would need some more information regarding what you are trying to do. This is not much to go on. The error has something to do with the previous update statement (before line 314).

What version of DBbrowser are you using?

sudomax commented 3 months ago

DBBrowser Version 3.12.2 on Mac

I've tried with those 4 files :

This is the code that I change before execution:

/*                          name                                sensor_id correction cutoff_new_meter cutoff_invalid_value */
INSERT INTO SENSORS VALUES ('sensor_id_gas',                    11,        1.0,    25.0,            1000.0); /* Change */
INSERT INTO SENSORS VALUES ('sensor_id_elec_feed_in_tariff_1',  3,        1.0,    25.0,            1000.0); /* Change */
INSERT INTO SENSORS VALUES ('sensor_id_elec_feed_out_tariff_1', 6,        1.0,    25.0,            1000.0); /* Change */
INSERT INTO SENSORS VALUES ('sensor_id_water',                  2,      1.0,    25.0,            1000.0); /* Change */
sudomax commented 3 months ago

Executed the line alone and got the following error:

UNIQUE constraint failed: statistics.metadata_id, statistics.start_ts

Looks like there are duplicate ts inside of STATS_NEW which might be the cause of the error.

Also line 238 of the script, looks like it should be SELECT value, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true instead of SELECT new_sum, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true

sudomax commented 3 months ago
image
patrickvorgers commented 3 months ago

It seems like that you had an earlier failed run and did not revert any changes. Could you start with the original database and perform the steps again.

patrickvorgers commented 3 months ago

In case you like you can also share the database and the csv files with me so that I can test it for you and see whether I can reproduce the error.

patrickvorgers commented 3 months ago

Executed the line alone and got the following error:

UNIQUE constraint failed: statistics.metadata_id, statistics.start_ts

Looks like there are duplicate ts inside of STATS_NEW which might be the cause of the error.

Also line 238 of the script, looks like it should be SELECT value, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true instead of SELECT new_sum, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true

Normally the new_sum is the same as value unless you had a meter change in the imported data. To cover that case aswell I will update the script accordingly.

sudomax commented 3 months ago

new_sum is not the same as value for me. I moved in into my house while the meter already had a couple kWh. The value starts at x kWh while the sum starts at 0.

The temp.new_stats table contains data from HA and the new values. The insert statement fails for me because it already has entries in the statisticstable for common value of ts. I've tried to delete all of the entries from statistics after creating temp.new_stats and before executing the insert statement. I also had to remove the ON CONFLICT DO UPDATE SET sum = excluded.sum part because of the syntax error. Then only it works.

I don't think the data I'm using has anything special in it, I've reduced the values to the bare minimum for testing purpose and I observe the same.

patrickvorgers commented 3 months ago

The script copies the existing data from the statistics table for the specified sensor into the STATS_NEW table. After calculating the new sum it copies the new records and updates any existing records in the statistics table. By removing the ÒN CONFLICT the existing records are not updated anymore which results in incorrect values.

I checked your screenshot and saw that you have rows with NULL values for the sensor_id column for the STATS_NEW table. This should not be possible and that is causing the issue further down the road in the script. We have to determine why you have NULL values in the STATS_NEW table.

You could try to debug and execute a part of the script and check at what point the STATS_NEW table is filled with NULL values.

sudomax commented 3 months ago

"""I checked your screenshot and saw that you have rows with NULL values for the sensor_id column for the STATS_NEW table. This should not be possible and that is causing the issue further down the road in the script. We have to determine why you have NULL values in the STATS_NEW table""" That was the first time around. After dropping the table and trying what I said last, it worked.

I know removing the ON CONFLICT will not update the existing record, that's why I preemptively remove them from the db prior to inserting them. The ON CONFLICT part still gives a syntax error.

On Mon, Jun 3, 2024, 23:14 Patrick Vorgers @.***> wrote:

The script copies the existing data from the statistics table for the specified sensor into the STATS_NEW table. After calculating the new sum it copies the new records and updates any existing records in the statistics table. By removing the ÒN CONFLICT the existing records are not updated anymore which results in incorrect values.

I checked your screenshot and saw that you have rows with NULL values for the sensor_id column for the STATS_NEW table. This should not be possible and that is causing the issue further down the road in the script. We have to determine why you have NULL values in the STATS_NEW table.

— Reply to this email directly, view it on GitHub https://github.com/patrickvorgers/Home-Assistant-Import-Energy-Data/issues/33#issuecomment-2146134931, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOMVQVM2O5RASXGVFZRW5Y3ZFTMCDAVCNFSM6AAAAABIVEZBKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBWGEZTIOJTGE . You are receiving this because you authored the thread.Message ID: <patrickvorgers/Home-Assistant-Import-Energy-Data/issues/33/2146134931@ github.com>

patrickvorgers commented 3 months ago

Okay good to read that you found a workaround.

I updated the script to make it more robust, handling invalid data (null values, duplicate timestamps). This may resolve your issue. For now I will close the issue.

sudomax commented 3 months ago

The ON CONFLICT DO part is still giving me a syntax error. Any way you could try to repro with a SQLite db?

patrickvorgers commented 3 months ago

I tested with my own SQLite db on windows and cannot reproduce the issue. Can you share your copy of your SQLite db and CSV files so that I can try to reproduce the issue?

Can you also check the SQLite Version (DB Browser -> Help -> About)?

image

Based on the SQLite documentation the version should be 3.35.0 or higher (see below).

UPSERT syntax was added to SQLite with version 3.24.0 (2018-06-04). The original implementation closely followed the PostgreSQL syntax in that it only permitted a single ON CONFLICT clause and it required a conflict target for on DO UPDATE. The syntax was generalized to permit multiple ON CONFLICT clauses and to allow DO UPDATE resolution without a conflict target in SQLite version 3.35.0 (2021-03-12).

You could try to specify the unique index explicitly to determine the conflict (supported since version 3.24.0).

INSERT INTO statistics (state, sum, metadata_id, created_ts, start_ts)
SELECT value, new_sum, sensor_id, ts, ts FROM STATS_NEW WHERE true
ON CONFLICT (metadata_id, start_ts) DO UPDATE SET sum = excluded.sum;
patrickvorgers commented 3 months ago

@sudomax did you manage to test the above?

sudomax commented 3 months ago

Still the same error. I'm not comfortable sharing the database, feel free to close the issue. Thank you