FrameworkBenchmarks icon indicating copy to clipboard operation
FrameworkBenchmarks copied to clipboard

/updates should be validated

Open synopse opened this issue 2 years ago • 5 comments

OS (Please include kernel version)

All

Expected Behavior

Calling /updates should actually update the database for all frameworks. The TFB validation bot should validate this expected behavior.

Actual Behavior

The TFB code does not validate that /updates did actually update the expected rows.

Steps to reproduce behavior

  • Run /updates?queries=10 on a buggy server
  • Output is something like [{"id":8744,"randomNumber":4603},{"id":2770,"randomNumber":1540},...
  • The first half ids are correctly modified, but the second half was not correctly updated.
  • A typical bug is to have a wrong UPDATE statement.

IMHO the TFB validator should actually check that select randomNumber from worlds where id=8744 is actually 4603, for all returned pairs of the JSON array.

Other details and logs

This won't affect performance measurement, because the same number of updates are done from a random number. But it should help test correct code. :)

synopse avatar Mar 02 '23 11:03 synopse

@fafhrd91 - FYI

pavelmash avatar Mar 02 '23 16:03 pavelmash

it is not clear why it should be pl+=2 instead of 1, it is placeholder position and 2 would create references outside of passed parameters. i would suggest to provide generated query and show why it is wrong.

fafhrd91 avatar Mar 02 '23 17:03 fafhrd91

@fafhrd91 And with pl += 1 you generated

update world set randomNumber = CASE id when $1 then $2 when $3 then $4 ...
... when $9 then $10 else randomNumber end where id in ($1,$2,$3,$4,$5)

Then only the first half IDs are corrects, and you modify unexpected IDs.

The expected syntax is AFAIK

update world set randomNumber = CASE id when $1 then $2 when $3 then $4 ...
... when $9 then $10 else randomNumber end where id in ($1,$3,$5,$7,$9)

This is why pl += 2 seems to be needed IMHO. Just check the DB after one /updates call.

synopse avatar Mar 02 '23 17:03 synopse

it is not what get generated, pl is not get reseted between two loops

fafhrd91 avatar Mar 02 '23 17:03 fafhrd91

@fafhrd91 You are right. My bad.

But the main idea of this issue is still valid. I did made the generation error on my side, and found out that the /updates should be verified because errors happen.

synopse avatar Mar 02 '23 19:03 synopse