Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

[Warn] Conflict of codegen methods #569

Open
Devjiu opened this issue Jul 5, 2023 · 2 comments · May be fixed by #575
Open

[Warn] Conflict of codegen methods #569

Devjiu opened this issue Jul 5, 2023 · 2 comments · May be fixed by #575
Assignees
Labels
help wanted Extra attention is needed invalid This doesn't seem right

Comments

@Devjiu
Copy link
Contributor

Devjiu commented Jul 5, 2023

I am not sure how to resolve this clash of namings.

ReductionCode ResultSetReductionJIT::codegen() const

vs

ReductionCode GpuReductionHelperJIT::codegen(const CompilationOptions& co) 

at omniscidb/QueryEngine/ResultSetReductionJIT.h

Second one method have single call, if it's what's intended to be done, let's remove virtual and rename GpuReductionHelperJIT method.

@Devjiu Devjiu added help wanted Extra attention is needed invalid This doesn't seem right labels Jul 5, 2023
@alexbaden
Copy link
Contributor

Can we add the unused CompilationOptions argument to the virtual method?

@Devjiu
Copy link
Contributor Author

Devjiu commented Jul 5, 2023

Can we add the unused CompilationOptions argument to the virtual method?

Yes, this is one option, but I don't understand the GpuReductionHelperJIT class. As far as I understand it doesn't work as an inherited class at the moment. So if it's just composition and the GpuReductionHelperJIT needs to be used in a specific place, we can leave the ResultSetReductionJIT interface unchanged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants