keda icon indicating copy to clipboard operation
keda copied to clipboard

Postgres Scaler fails when the postgresql query returns NULL value

Open C-Ravina opened this issue 4 months ago • 4 comments

Report

I deployed a scaledobject yaml with PostgreSQL scaler. If the query I use is select avg(salary) from employees and there are no rows present in the employees table then avg(salary) is NULL. When the postgresql query returns null value, then keda stops functioning with the following error. KEDA ScalerFailed error inspecting postgreSQL: could not query postgreSQL: sql: Scan error on column index 0,name "avg": converting NULL to float64 is unsupported

Expected Behavior

Keda should accept the NULL value returned by the postgreSQL query as 0 and so should scale the target cr to zero pods.

Actual Behavior

Keda operator returns an error and scaling fails.

Steps to Reproduce the Problem

  1. Configure keda scaledobject with postgreSQl trigger.
  2. Delete all values in the table being referred by the postgreSQL query in scaledobject yaml.
  3. Check the logs of keda operator or describe the scaledobject created, to check the error.

Logs from KEDA operator

KEDA ScalerFailed  error inspecting postgreSQL: could not query postgreSQL: sql: Scan error on column index 0,name "avg": converting NULL to float64 is unsupported

KEDA Version

2.13.1

Kubernetes Version

1.26

Platform

Other

Scaler Details

PostgreSQL

Anything else?

No response

C-Ravina avatar Apr 12 '24 08:04 C-Ravina

Hi, the issue is here: https://github.com/kedacore/keda/blob/main/pkg/scalers/postgresql_scaler.go#L171. Using NullFloat64 may allow a check against null after the scan.

Also, worth noting that you might be to able to use the COALESCE function in your query to return 0 in the case of null in the first instance. SELECT COALESCE(avg, 0) FROM ...

I can take this issue; but should the user be able to select the behaviour they desire, either fail or return a default value. Also if coalesce works, this might just be an item to add to the docs, instead of a change to the scaler. Will wait for maintainers to comment here.

robpickerill avatar Apr 13 '24 18:04 robpickerill

Hi, the issue is here: https://github.com/kedacore/keda/blob/main/pkg/scalers/postgresql_scaler.go#L171. Using NullFloat64 may allow a check against null after the scan.

Also, worth noting that you might be to able to use the COALESCE function in your query to return 0 in the case of null in the first instance. SELECT COALESCE(avg, 0) FROM ...

I can take this issue; but should the user be able to select the behaviour they desire, either fail or return a default value. Also if coalesce works, this might just be an item to add to the docs, instead of a change to the scaler. Will wait for maintainers to comment here.

Hi, the mishandling of null values from the db queries will also occur in the following places:

  • MySQL: https://github.com/kedacore/keda/blob/main/pkg/scalers/mysql_scaler.go#L208
  • Mssql : https://github.com/kedacore/keda/blob/main/pkg/scalers/mssql_scaler.go#L259

I guess this should just be handled by sql.NullFloat64 like @robpickerill mentioned, or by using a *float64 to check for the nil case.

ayoyu avatar Apr 27 '24 17:04 ayoyu

I'm not totally sure if KEDA should accept NULL as 0, or at least as default, because NULL is a failure indeed. SQL engines support handling this within the query and there you have the full control without having to include any extra parameter. @tomkerkhove @zroubalik , opinions?

JorTurFer avatar May 07 '24 20:05 JorTurFer

I agree, I think that it would be better to add a note to the documentation instead.

zroubalik avatar May 17 '24 09:05 zroubalik