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

pebosi’s picture

Status:Active» Needs review
heyrocker’s picture

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.

Dries’s picture

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

pebosi’s picture

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

Dries’s picture

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

pebosi’s picture

Ok the first patch was dumb :)

moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

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

moshe weitzman’s picture

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

pebosi’s picture

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

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

andypost’s picture

Suppose 'status' condition should be prefixed too.

andypost’s picture

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

Dries’s picture

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.

mvaneyken’s picture

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!

neoglez’s picture

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.

neoglez’s picture

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.

marcingy’s picture

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

Needs to be fixed in d8 first.

andypost’s picture

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

catch’s picture

hswong3i’s picture

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.

webchick’s picture

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.

neoglez’s picture

(...) 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".

webchick’s picture

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.

salvis’s picture

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.

deggertsen’s picture

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

rbayliss’s picture

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

andypost’s picture

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

kiam’s picture

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

AdamGS’s picture

subscribe

Jeremy’s picture

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.

dwalker51’s picture

patch at #16 worked for me.

neoglez’s picture

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.

neoglez’s picture

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.

salvis’s picture

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

sarah_p’s picture

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.

neoglez’s picture

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!

neoglez’s picture

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

neoglez’s picture

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.

neoglez’s picture

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.

neoglez’s picture

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.

neoglez’s picture

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

Status:Needs work» Needs review
andypost’s picture

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

chx’s picture

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.

catch’s picture

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.

chx’s picture

Status:Needs review» Reviewed & tested by the community

Sure it is.

webchick’s picture

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

neoglez’s picture

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

webchick’s picture

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

embeepea’s picture

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.

webchick’s picture

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

neoglez’s picture

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

deanflory’s picture

Issue summary:View changes

Resaving the User: name field in Views worked for me to get beyond the bug in D7.27.