search

dmwm / DBSClient

Apache License 2.0
3 stars 8 forks source link

Add aggregation to parentDSTrio API #33

Closed vkuznet closed 3 years ago

yuyiguo commented 3 years ago

@vkuznet Please check the comments on the code.

vkuznet commented 3 years ago

@yuyiguo I think you did not commit comments as I don't see them here in GitHub.

yuyiguo commented 3 years ago

@vkuznet Did you see it now?

vkuznet commented 3 years ago

No, I don't see any comments in github web interface.

yuyiguo commented 3 years ago

I don't know what went wrong, but Let me put my comment here. In function def testiAggParentDSTrio(self), you had something like:

{"l":29838,"pfid":653591317,"r":98}
{"l":26422,"pfid":653591317,"r":98}

Are the pfids db IDs? if so, the test will fail when using a different DB.

vkuznet commented 3 years ago

@yuyiguo now I can see your comment. So, the pfid is what ORACLE query generates and is referring to parent file id. Of course they change but the unit tests does not use ANY DBs and only checks the aggParentDSTrio functionality which transforms one results set into another. As such I can put any numbers and test will work. If you want I can change the data and put some fake numbers, I only used output from APIs to test the logic.

yuyiguo commented 3 years ago

That is fine. Any number should work as long as it does not retrieve from DB again.