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

Files: 
CommentFileSizeAuthor
#50 766382_49.patch5.03 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 33,544 pass(es).
[ View ]
#49 766382_49_test_only.patch3.95 KBchx
FAILED: [[SimpleTest]]: [MySQL] 33,532 pass(es), 2 fail(s), and 8 exception(es).
[ View ]
#49 766382_49.patch5.04 KBchx
PASSED: [[SimpleTest]]: [MySQL] 33,558 pass(es).
[ View ]
#46 test-book-block-on-access-module-3.patch5.05 KBneoglez
PASSED: [[SimpleTest]]: [MySQL] 33,549 pass(es).
[ View ]
#44 test-book-block-on-access-module-2.patch4.61 KBneoglez
FAILED: [[SimpleTest]]: [MySQL] 33,531 pass(es), 1 fail(s), and 4 exception(es).
[ View ]
#41 test-book-block-on-access-module-1.patch4.61 KBneoglez
FAILED: [[SimpleTest]]: [MySQL] 33,419 pass(es), 2 fail(s), and 8 exception(es).
[ View ]
#37 patch16n33.patch3.39 KBsarah_p
FAILED: [[SimpleTest]]: [MySQL] 33,433 pass(es), 3 fail(s), and 0 exception(es).
[ View ]
#33 test-book-block-on-access-module.patch3.2 KBneoglez
FAILED: [[SimpleTest]]: [MySQL] 33,427 pass(es), 4 fail(s), and 7 exception(es).
[ View ]
#16 fixed-column-is-ambigous-book-module.patch602 bytesneoglez
PASSED: [[SimpleTest]]: [MySQL] 33,460 pass(es).
[ View ]
#12 766382-menu-follow-d7.patch683 bytesandypost
PASSED: [[SimpleTest]]: [MySQL] 20,927 pass(es).
[ View ]
#4 766382_1.patch785 bytespebosi
FAILED: [[SimpleTest]]: [MySQL] 20,447 pass(es), 312 fail(s), and 166 exception(es).
[ View ]
#4 766382_2.patch818 bytespebosi
PASSED: [[SimpleTest]]: [MySQL] 20,833 pass(es).
[ View ]
drupal-menu.patch681 bytespebosi
PASSED: [[SimpleTest]]: [MySQL] 19,103 pass(es).
[ View ]

Comments

Status:Active» Needs review

Status:Needs review» Reviewed & tested by the community

This 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.

Is 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'?

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new818 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,833 pass(es).
[ View ]
new785 bytes
FAILED: [[SimpleTest]]: [MySQL] 20,447 pass(es), 312 fail(s), and 166 exception(es).
[ View ]

Ok, i did forget prefixing...created two patches with two different possibilities, please review again

regards

I like 766382_2.patch best but let's see. :)

Ok the first patch was dumb :)

Status:Needs review» Reviewed & tested by the community

hmmm. 0 passes with 766382_2. i'll try a re-test. code looks good.

#4: 766382_2.patch queued for re-testing.

#4: 766382_2.patch queued for re-testing.

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Suppose 'status' condition should be prefixed too.

Status:Fixed» Needs review
StatusFileSize
new683 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,927 pass(es).
[ View ]

Resulting query

SELECT n.nid AS nid
FROM
{node} n
WHERE  (status = :db_condition_placeholder_0) AND (n.nid IN  (:db_condition_placeholder_1))

So status should be prefixed too

Status:Needs review» Fixed

You're right, Andy. Committed to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Problem 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!

Status:Closed (fixed)» Needs review
StatusFileSize
new602 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,460 pass(es).
[ View ]

I 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.

Priority:Normal» Major

Mm, by the way, i think this is major:

Issues which have significant repercussions but do not render the whole system unusable are marked major. An example would be a PHP error which is only triggered under rare circumstances or which affects only a small percentage of all users. These issues are prioritized in the current development release and backported to stable releases where applicable. Major issues do not block point releases.

Version:7.x-dev» 8.x-dev
Issue tags:+needs backport to D7

Needs to be fixed in d8 first.

Are this affected by "node access queries to 7.3" ? http://drupal.org/node/1204572

Status:Needs review» Reviewed & tested by the community

Confirm patch from #16 can apply to both D7/8 and able to solve the problem with node access.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

I 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.

(...) because it didn't come with tests in the first place.

You mean that the people should test it or write a test for this use case??
...couldn't find a description of "Needs test".

Sorry, 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.

Marked #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: DBTNG: Should refuse unqualified column names in condition() clauses, which proposes a more effective approach.

Patch in #16 fixed this problem on my site. Thanks!

@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?).

Core already has node_access_test.module, suppose this should be extended a bit

I have marked #1057770: SQL error when trying to view book as duplicate of this report.

subscribe

Subscribing, 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.

patch at #16 worked for me.

Component:menu system» database system
Status:Needs work» Needs review
StatusFileSize
new3.2 KB
FAILED: [[SimpleTest]]: [MySQL] 33,427 pass(es), 4 fail(s), and 7 exception(es).
[ View ]

@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.

Status:Needs review» Needs work

The last submitted patch, test-book-block-on-access-module.patch, failed testing.

The last submitted patch, test-book-block-on-access-module.patch, failed testing.

Well, that's the expected result, isn't it? ;-)
View details for more info.

Exactly. Now post a patch with #16 and #33 combined; and set 'needs review' again.

Status:Needs work» Needs review
StatusFileSize
new3.39 KB
FAILED: [[SimpleTest]]: [MySQL] 33,433 pass(es), 3 fail(s), and 0 exception(es).
[ View ]

This is a merged version of the patches in 16 and 33.

Status:Needs review» Needs work

The last submitted patch, patch16n33.patch, failed testing.

mm, 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!

Status:Needs review» Needs work

Aha! 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'));

StatusFileSize
new4.61 KB
FAILED: [[SimpleTest]]: [MySQL] 33,419 pass(es), 2 fail(s), and 8 exception(es).
[ View ]

Well, 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.

Status:Needs work» Needs review

Update status

Status:Needs review» Needs work

The last submitted patch, test-book-block-on-access-module-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.61 KB
FAILED: [[SimpleTest]]: [MySQL] 33,531 pass(es), 1 fail(s), and 4 exception(es).
[ View ]

Oops...

The last submitted patch, test-book-block-on-access-module-2.patch, failed testing.

StatusFileSize
new5.05 KB
PASSED: [[SimpleTest]]: [MySQL] 33,549 pass(es).
[ View ]

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

I think this patch is great to be commited, the whitespace can be fixed latter

+++ b/modules/book/book.testundefined
@@ -274,6 +277,12 @@ class BookTestCase extends DrupalWebTestCase {
+     ¶

A trailing whitespace

StatusFileSize
new5.04 KB
PASSED: [[SimpleTest]]: [MySQL] 33,558 pass(es).
[ View ]
new3.95 KB
FAILED: [[SimpleTest]]: [MySQL] 33,532 pass(es), 2 fail(s), and 8 exception(es).
[ View ]

Or we can remove that. I also have attached the test only version to see it failing.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new5.03 KB
PASSED: [[SimpleTest]]: [MySQL] 33,544 pass(es).
[ View ]

Comments 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.

Status:Needs review» Reviewed & tested by the community

Sure it is.

Status:Reviewed & tested by the community» Fixed

Awesome. 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! :)

Thanks for the acknowledgement!! ...very motivating :)

No problem at all! Thanks for the patch! :D

I'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.

can you see if you get the same error with 7.x-dev?

@mark.phillips the same problem described in this issue appears (apparently) all over core (see #25 and specially #1228214: DBTNG: Should refuse unqualified column names in condition() clauses), but at least for this specific case (book module) the issue should be resolved in 7.x and 8.x (#49).

Status:Fixed» Closed (fixed)
Issue tags:-Needs tests, -needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.