Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add selector name to getValue() and getValues() #24

Closed
wants to merge 1 commit into from

Conversation

lsmith77
Copy link
Member

@lsmith77 lsmith77 commented Dec 9, 2011

I was working on implementing support for selectors into Jackalope when I stumbled over this. Since column names are only unique for each selector the current API makes no sense. As such I added a selector parameter (if none is supplied it would fallback to the default selector just like for the other methods).

see also jackalope/jackalope#53

@lsmith77
Copy link
Member Author

lsmith77 commented Dec 9, 2011

nevermind .. i guess the intention is to make the column names unique by qualifying them with the selector name aka "foo.bar"

@lsmith77 lsmith77 closed this Dec 9, 2011
@dbu
Copy link
Member

dbu commented Dec 11, 2011

@lsmith77: sorry, but i strongly disagree with this. we diverge from jcr for no good reason.

i just ran the following code in the JavaDavexClient:

QueryManager qm = s.getWorkspace().getQueryManager();
Query query = qm.createQuery("SELECT * FROM [nt:file] INNER JOIN [nt:folder] AS second ON [nt:file].[jcr:createdBy] = second
QueryResult r = query.execute();
String[] names = r.getColumnNames();
for (String n: names) {
    System.out.println(n);
}

and the output is

nt:file.jcr:createdBy
nt:file.jcr:created
nt:file.jcr:primaryType
second.jcr:createdBy
second.jcr:created
second.jcr:primaryType

so the right way to handle this is that as soon as there is a join, the column name contains the selector name. the jcr spec is not overly precise and explicit there, i guess the overprudent client would use the getColumns to know what names to use, or just getValues and explicitly select fields so he knows what he wants by position instead of key name.

can you please undo this change?

@lsmith77
Copy link
Member Author

i never merged the changes .. i closed the PR

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.

None yet

2 participants