Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
hi,
tried using domain module, but getting this error in menu_tree_check_access():
Error message
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'nid' in where clause is ambiguous: SELECT DISTINCT node.nid AS nid FROM {node} node INNER JOIN {domain_access} da_admin ON da_admin.nid = node.nid WHERE (status = :db_condition_placeholder_0) AND (nid IN (:db_condition_placeholder_1)) AND(( (da_admin.gid = :db_condition_placeholder_2) AND (da_admin.realm = :db_condition_placeholder_3) )OR( (da_admin.gid = :db_condition_placeholder_4) AND (da_admin.realm = :db_condition_placeholder_5) )); Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => 138 [:db_condition_placeholder_2] => 0 [:db_condition_placeholder_3] => domain_site [:db_condition_placeholder_4] => 0 [:db_condition_placeholder_5] => domain_id ) in menu_tree_check_access() (line 1285 of /drupal7/includes/menu.inc).
changing
$select->condition('nid', $nids, 'IN');
to
$select->condition('node.nid', $nids, 'IN');
worked for me. mini patch attached, please review
regards
Comment | File | Size | Author |
---|---|---|---|
#50 | 766382_49.patch | 5.03 KB | catch |
#49 | 766382_49_test_only.patch | 3.95 KB | chx |
#49 | 766382_49.patch | 5.04 KB | chx |
#46 | test-book-block-on-access-module-3.patch | 5.05 KB | neoglez |
#44 | test-book-block-on-access-module-2.patch | 4.61 KB | neoglez |
Comments
Comment #1
pebosi CreditAttribution: pebosi commentedComment #2
gddThis can happen with node_access queries that bring in other tables that also have the column 'nid'. This patch works and still applies, and it is the right solution.
Comment #3
Dries CreditAttribution: Dries commentedIs it the right solution? What if we have database prefixing enabled? Shouldn't we create an alias for the node table, say 'n', and write 'n.nid'?
Comment #4
pebosi CreditAttribution: pebosi commentedOk, i did forget prefixing...created two patches with two different possibilities, please review again
regards
Comment #5
Dries CreditAttribution: Dries commentedI like 766382_2.patch best but let's see. :)
Comment #6
pebosi CreditAttribution: pebosi commentedOk the first patch was dumb :)
Comment #7
moshe weitzman CreditAttribution: moshe weitzman commentedhmmm. 0 passes with 766382_2. i'll try a re-test. code looks good.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman commented#4: 766382_2.patch queued for re-testing.
Comment #9
pebosi CreditAttribution: pebosi commented#4: 766382_2.patch queued for re-testing.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #11
andypostSuppose 'status' condition should be prefixed too.
Comment #12
andypostResulting query
So status should be prefixed too
Comment #13
Dries CreditAttribution: Dries commentedYou're right, Andy. Committed to CVS HEAD. Thanks.
Comment #15
mvaneyken CreditAttribution: mvaneyken commentedProblem appears to have resurfaced with Drupal core 7.2.
Situation:
Am able to list & click on book title as a registered user (site administrator). As anonymous user, I can list, but not click on the book titles. Clicking on the book title results in:
PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'node.nid' in 'where clause': SELECT DISTINCT n.title AS title FROM {node} n INNER JOIN {node_access} na ON na.nid = n.nid WHERE (node.nid = :db_condition_placeholder_0) AND (n.language IN (:db_condition_placeholder_1, :db_condition_placeholder_2)) AND(( (na.gid = :db_condition_placeholder_3) AND (na.realm = :db_condition_placeholder_4) )OR( (na.gid = :db_condition_placeholder_5) AND (na.realm = :db_condition_placeholder_6) ))AND (na.grant_view >= :db_condition_placeholder_7) ; Array ( [:db_condition_placeholder_0] => 33 [:db_condition_placeholder_1] => en [:db_condition_placeholder_2] => und [:db_condition_placeholder_3] => 0 [:db_condition_placeholder_4] => all [:db_condition_placeholder_5] => 0 [:db_condition_placeholder_6] => abt [:db_condition_placeholder_7] => 1 ) in book_block_view() (line 292 of /home/consult/public_html/modules/book/book.module).
Fix:
Changed book.module, function book_block_view($delta = ''), condition('nid') to condition('n.nid')
IE:
...
elseif ($current_bid) {
// Only display this block when the user is browsing a book.
// MVE test patch:
// $select = db_select('node', 'n')
// ->fields('n', array('title'))
// ->condition('nid', $node->book['bid'])
// ->addTag('node_access');
$select = db_select('node', 'n')
->fields('n', array('title'))
->condition('n.nid', $node->book['bid'])
->addTag('node_access');
$title = $select->execute()->fetchField();
...
Note to maintainers: Thank you for your efforts in writing the code and maintaining this log. It's a great help!
Comment #16
neoglez CreditAttribution: neoglez commentedI could open another issue but since the title is also valid for this one i'm posting the patch here.
Patch described in #15, this time for book module, it's of the same nature of patch in #12 (commited), i tested & it seems to work fine.
Comment #17
neoglez CreditAttribution: neoglez commentedMm, by the way, i think this is major:
Comment #18
marcingy CreditAttribution: marcingy commentedNeeds to be fixed in d8 first.
Comment #19
andypostAre this affected by "node access queries to 7.3" ? http://drupal.org/node/1204572
Comment #20
catch#1013864: Book navigation block fails with Node Access modules had exactly the same patch.
Comment #21
hswong3i CreditAttribution: hswong3i commentedConfirm patch from #16 can apply to both D7/8 and able to solve the problem with node access.
Comment #22
webchickI had the other issue tagged needs tests, because we need them. The issue that got marked duplicate had a patch committed that caused the problem that this patch is fixing, because it didn't come with tests in the first place.
Comment #23
neoglez CreditAttribution: neoglez commentedYou mean that the people should test it or write a test for this use case??
...couldn't find a description of "Needs test".
Comment #24
webchickSorry, they should write an automated test for this use case. Something that would hit this section of code and cause the test to fail without the patch, and passes with it.
Comment #25
salvisMarked #1227860: PDOException: Integrity constraint violation: ambiguous nid in book.module with node_access as a duplicate of this.
There are other similar cases, e.g. in blog.module, forum.module, even node.module, with unqualified column names like nid/uid/status/etc. in condition() calls.
@webchick: Once we have the table alias in place it's unlikely to get lost again — I'm not sure just how worthwhile a test is here, but in order to not derail this issue I've created #1228214: Database queries should refuse unqualified column names in condition() clauses or default to the base table, which proposes a more effective approach.
Comment #26
deggertsen CreditAttribution: deggertsen commentedPatch in #16 fixed this problem on my site. Thanks!
Comment #27
rbayliss CreditAttribution: rbayliss commented@webchick: In this case, you have a module providing node grants in order to trigger this error. I don't see how we can write a test case for this as there are no node_grants providers in core (or am I wrong?).
Comment #28
andypostCore already has node_access_test.module, suppose this should be extended a bit
Comment #29
Anonymous (not verified) CreditAttribution: Anonymous commentedI have marked #1057770: SQL error when trying to view book as duplicate of this report.
Comment #30
AdamGS CreditAttribution: AdamGS commentedsubscribe
Comment #31
Jeremy CreditAttribution: Jeremy commentedSubscribing, as was reported against the support module #1249788: unexpected error in book module when enabling support ticketing module in 7.x but is a bug in the book module.
Comment #32
dwalker51 CreditAttribution: dwalker51 commentedpatch at #16 worked for me.
Comment #33
neoglez CreditAttribution: neoglez commented@webnick here is the test.
As usual there are more than one approach but i think this one of the less painfull.
By applying this patch to the test one can run the two cases: with patch in #16 and without and see the exceptions (beautifully) thrown.
The patch is against D8.
Comment #35
neoglez CreditAttribution: neoglez commentedWell, that's the expected result, isn't it? ;-)
View details for more info.
Comment #36
salvisExactly. Now post a patch with #16 and #33 combined; and set 'needs review' again.
Comment #37
sarah_p CreditAttribution: sarah_p commentedThis is a merged version of the patches in 16 and 33.
Comment #39
neoglez CreditAttribution: neoglez commentedmm, at least we don't have more exceptions.
I see the reason of the fail in BookTestCase->testNavigationBlockOnAccessModuleEnabled() is becouse i forgot to give anonymous users the 'node test view' permission, the other two fails in BookTestCase->testBookNavigationBlock() are 'new'.
@sarah_p Thanks for the merge!
Comment #40
neoglez CreditAttribution: neoglez commentedAha! The $this->admin_user needs also 'node test view' permission, something like:- $this->admin_user = $this->drupalCreateUser(array('create new books', 'create book content', 'edit own book content', 'add content to books', 'administer blocks'));+ $this->admin_user = $this->drupalCreateUser(array('create new books', 'create book content', 'edit own book content', 'add content to books', 'administer blocks', 'node test view'));
Comment #41
neoglez CreditAttribution: neoglez commentedWell, actually no.
Let's try this one althoug i think it might make sense to implement another class to (really) separate the cases when an access module is enabled.
Comment #42
neoglez CreditAttribution: neoglez commentedUpdate status
Comment #44
neoglez CreditAttribution: neoglez commentedOops...
Comment #46
neoglez CreditAttribution: neoglez commentedComment #47
neoglez CreditAttribution: neoglez commentedComment #48
andypostI think this patch is great to be commited, the whitespace can be fixed latter
A trailing whitespace
Comment #49
chx CreditAttribution: chx commentedOr we can remove that. I also have attached the test only version to see it failing.
Comment #50
catchComments weren't up to coding standards, here's an edited patch, if this is RTBC again when I'm back off my flight tomorrow I'm happy to commit otherwise.
Comment #51
chx CreditAttribution: chx commentedSure it is.
Comment #52
webchickAwesome. Great work on this folks! I'm sorry about being a stickler about tests, but hopefully this stays fixed now. :)
Committed and pushed to 8.x and 7.x. Yay for major bugs biting the dust! :)
Comment #53
neoglez CreditAttribution: neoglez commentedThanks for the acknowledgement!! ...very motivating :)
Comment #54
webchickNo problem at all! Thanks for the patch! :D
Comment #55
embeepea CreditAttribution: embeepea commentedI'm not sure if this is related to this issue or not, but I'm seeing a very similar ambiguous 'nid' column error using D7.8 with
Entity Reference 7.x-1.0-beta1, together with either Organic Groups Access Control (7.x-1.1) or Content Access (7.x-1.2-beta1). I am not using the Book module at all. The error occurrs when either of these content access modules is enabled, but does not happen when both are disabled. #1309704: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1052 has more details. I just though I'd post a note here too in case anyone from this discussion can offer any insights, since the issue seems similar.
Comment #56
webchickcan you see if you get the same error with 7.x-dev?
Comment #57
neoglez CreditAttribution: neoglez commented@mark.phillips the same problem described in this issue appears (apparently) all over core (see #25 and specially #1228214: Database queries should refuse unqualified column names in condition() clauses or default to the base table), but at least for this specific case (book module) the issue should be resolved in 7.x and 8.x (#49).
Comment #60
deanflory CreditAttribution: deanflory commentedResaving the User: name field in Views worked for me to get beyond the bug in D7.27.