Skip to content

Conversation

@msullivan
Copy link
Member

@msullivan msullivan commented Aug 29, 2025

Following Elvis's original branch, I added it to a gel.qb module.
I also, on Elvis's suggestion, modified std to re-export everything
from gel.qb.

I renamed his foreach to for_ to better match EdgeQL.

Fixes #729.

@msullivan msullivan requested review from dnwpark and elprans August 29, 2025 20:09
# SPDX-FileCopyrightText: Copyright Gel Data Inc. and the contributors.


"""Query building constructs"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for this to be public? If not, move this into _internal._qbmodel._abstract

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of felt that language features like this ought to be accessible somewhere other than the mildly-hacky std. Not sure if gel.qb is the best for that, though anyway.

I don't really mind dropping it if you think it's not worth exposing

gel/qb.py Outdated
_X = TypeVar("_X", bound=_abstract.GelType)


def for_in(iter: type[_T], body: Callable[[type[_T]], type[_X]]) -> type[_X]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a bikeshed about naming this and IIRC the conclusion was that for_ was acceptable (we already have and_ and if_ etc).

@msullivan msullivan changed the title Add a for_in operator Add a for_ operator Aug 29, 2025

def process(self, mod: IntrospectedModule) -> None:
if str(self.canonical_modpath) == 'std':
self.write("# Re-export top-level query-builder functions")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must self.py_file.export(name) and self.py_file.update_globals(names) here (I'd also move this block into prepare_namespace since it already has std-specific wrangling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that accomplish here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to emitting the explicit export?

Copy link
Contributor

@dnwpark dnwpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but will wait on those other pydantic comments from elvis

"PIE", # flake8-pie
"PL", # pylint
"PYI", # flake8-pyi
"Q", # flake8-quotes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha!

return (self.iter_expr, self.body)

def _edgeql(self, ctx: ScopeContext) -> str:
ctx.bind(self.var)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this get bound in for_?

iter_expr = _qb.edgeql_qb_expr(iterator)
scope = _qb.Scope()
var = _qb.Variable(type_=iter_expr.type, scope=scope)
body_ = body(_qb.AnnotatedVar(iterator.__gel_origin__, var)) # type: ignore [arg-type, attr-defined]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterator.gel_origin might be a pure type[GelModel] if you just passed an object type class, e.g for_(User, ...), so you need to check iterator is really a BaseAlias

@msullivan msullivan force-pushed the for-loops branch 3 times, most recently from 70098bc to 10b7da6 Compare September 4, 2025 21:24
@msullivan msullivan merged commit d93f90d into master Sep 4, 2025
43 checks passed
@msullivan msullivan deleted the for-loops branch September 4, 2025 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose a top-level for constructor for writing bulk queries

4 participants