Skip to:
Content

Opened 3 years ago

Last modified 12 days ago

#6350 reopened enhancement

XProfile field database schema

Reported by: johnjamesjacoby Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 1.0
Component: Extended Profile Keywords: 2nd-opinion needs-patch
Cc: espellcaste@…

Description

BuddyPress's XProfile database schema is... weird.

CREATE TABLE {$bp_prefix}bp_xprofile_fields (
	id bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY,
	group_id bigint(20) unsigned NOT NULL,
	parent_id bigint(20) unsigned NOT NULL,
	type varchar(150) NOT NULL,
	name varchar(150) NOT NULL,
	description longtext NOT NULL,
	is_required tinyint(1) NOT NULL DEFAULT '0',
	is_default_option tinyint(1) NOT NULL DEFAULT '0',
	field_order bigint(20) NOT NULL DEFAULT '0',
	option_order bigint(20) NOT NULL DEFAULT '0',
	order_by varchar(15) NOT NULL DEFAULT '',
	can_delete tinyint(1) NOT NULL DEFAULT '1',
	KEY group_id (group_id),
	KEY parent_id (parent_id),
	KEY field_order (field_order),
	KEY can_delete (can_delete),
	KEY is_required (is_required)
)

A few thoughts from staring at this too long:

  • Field options are just fields with a parent_id
  • Fields are just fields with a group_id
  • Groups are just fields with children

Change History (13)

#1 @johnjamesjacoby
3 years ago

A slightly more optimized schema might look something like:

CREATE TABLE {$bp_prefix}bp_xprofile_fields (
	id bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY,
	parent_id bigint(20) unsigned NOT NULL,
	type varchar(150) NOT NULL,
	name varchar(150) NOT NULL,
	description longtext NOT NULL,
	is_required tinyint(1) NOT NULL DEFAULT '0',
	is_default tinyint(1) NOT NULL DEFAULT '0',
	is_signup tinyint(1) NOT NULL DEFAULT '0',
	order bigint(20) NOT NULL DEFAULT '0',
	order_by varchar(15) NOT NULL DEFAULT '',
	can_delete tinyint(1) NOT NULL DEFAULT '1',
	KEY parent_id (parent_id),
	KEY order (order),
	KEY is_signup (is_signup),
	KEY is_required (is_required)
)

A few other thoughts:

  • We could move visibility out of field meta
  • We could eliminate the bp_xprofile_groups table, and have them instead be a field type group

#2 @boonebgorges
3 years ago

Can you say a bit more about the weirdness and how your changes are improvements? I don't fully understand your bullet points.

#3 @espellcaste
3 years ago

  • Cc espellcaste@… added

Indeed, what are the improvements this new model brings? It seems it's only organization but I'm not sure.

Also, you suggest moving visibility out of field meta, it might be obvious but current users making use of this would have to be thought of.

I like the idea of moving bp_xprofile_groups from a table to a field type, it seems a more organized way of looking at it.

Last edited 3 years ago by espellcaste (previous) (diff)

#4 @johnjamesjacoby
3 years ago

Improvements are initially organizational.

  • Eliminate dedicated table for field groups
  • Field groups become a type of field called group similar to option like we have now
  • The fields table currently has two duplicate columns for exactly the same use-case, only one is for fields and the other is for field-options, which are also technically just a custom field type
  • Code consolidation and simplification. What we have now is akin to having two separate database tables for both pages and posts. Because field groups, fields, and field options are basically the same things with different defined areas in the hierarchy, we can better represent that data structure in 1 table with fewer columns.

#5 follow-up: @DJPaul
3 years ago

JJJ, I think this looks OK, but I think it would help illustrate some of your bullet points if you could please provide a (sample) MySQL export of these tables so the rest of us can compare how the default xprofile group/field/data/values would relate to each other, and understand the proposed changes better (against our development databases, for example).

We could move visibility out of field meta

I don't see enough benefits to doing this -- unless you can suggest why BP would need to be able order fields by visibility when making an SQL query. I think the existing JOIN against the meta table for (what is) a key/value pair is fine; it only starts to be a hinderance when you try to order things by meta value.

#6 in reply to: ↑ 5 @johnjamesjacoby
3 years ago

Replying to DJPaul:

JJJ, I think this looks OK, but I think it would help illustrate some of your bullet points if you could please provide a (sample) MySQL export of these tables so the rest of us can compare how the default xprofile group/field/data/values would relate to each other, and understand the proposed changes better (against our development databases, for example).

Are you asking for a dump of data from the existing tables? If so, this seems like a strange request considering how easy it is to just create a few fields and groups in your own test installation.

Are you asking for a dump of data from new database tables? This seems less strange, though requires more work than I've put into this theory so far. I'm happy to do it, it just contradicts a bit with iterating publicly on trac (vs. creating huge amounts of code privately.) We shouldn't be working alone for weeks and then delivering new shiny; we should all feel free to experiment with rough ideas and help them take shape together.

We could move visibility out of field meta

I don't see enough benefits to doing this -- unless you can suggest why BP would need to be able order fields by visibility when making an SQL query. I think the existing JOIN against the meta table for (what is) a key/value pair is fine; it only starts to be a hinderance when you try to order things by meta value.

Ordering by visibility doesn't really make any sense in the context of profile fields, but it does open up the opportunity for more elaborate visibility queries without requiring a JOIN to data in the the unindexed meta_value column, but that's also not really the point of this exercise either IMO.

The rub is that the tables in this components schema no longer represent it's needs in the most accurate and efficient way; I'm proposing one possible alternative, of which there are infinite.

Imagine if post_status were meta data instead of an indexed column. Eventually, for profile fields to be speedy and increasingly complex, it would be an obvious improvement to move that out of meta data. Designating sign-up fields? Probably less-so, but it would be nice to concentrate a bit more on the experience of getting users into the community nicely rather than keeping them busy afterwards.

Since it sounds like the improvements are too vague to be easily imagined, I'll spend some time more fully implementing it. Expect me to iterate with several patches here.

#7 @DJPaul
3 years ago

Are you asking for a dump of data from new database tables?

Yes, or a just screenshot from something like phpMyAdmin. It would've helped me better visualise how/where the xprofile data fits into the proposed schema.

#8 @espellcaste
3 years ago

Are you asking for a dump of data from new database tables?

Yes, that would be helpful!

I quite still haven't understood the visual improvements it would bring. But I'm gonna wait for the patches and more development on it.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


3 years ago

This ticket was mentioned in Slack in #buddypress by jjj. View the logs.


2 years ago

#11 @DJPaul
22 months ago

  • Milestone Under Consideration deleted
  • Resolution set to invalid
  • Status changed from new to closed

This ticket hasn't seen any traction in a year, and we need more technical information in order to make a decision on this suggested change.

#12 @johnjamesjacoby
12 days ago

  • Keywords needs-patch added
  • Milestone set to 3.0
  • Priority changed from low to normal
  • Resolution invalid deleted
  • Severity changed from minor to normal
  • Status changed from closed to reopened
  • Type changed from defect (bug) to enhancement

Reopening for more discussion. See #6783 for more Xprofile schema changes related to this.

#13 @boonebgorges
12 days ago

The main improvement I see from @johnjamesjacoby's comment https://buddypress.trac.wordpress.org/ticket/6350#comment:4 is that "field groups" feels like an arbitrary object type. IMO it feels more like a taxonomy of fields than a field type itself (it's a way of grouping fields, and in theory it could be many-to-many - thus a taxonomy), but the general point seems right to me. If we were designing this component from scratch, knowing what we know now about how it's used, we'd certainly do something different from what actually exists.

Given that the component *does* exist, the question is, what do we do now? The amount of work required to design and implement the new schema is a small fraction of the overall labor required. We'd need an abstraction layer, so that queries by field-groups get translated into whatever new schema we have. We'd need a migration routine, which will need a bulk data migration tool #6841. The migrator is bound to be not-quite-100% efficient, which introduces support overhead. We'll cause breaks for people doing direct SQL queries or other advanced things with xprofile; we'll need publicity and documentation about this, and even then, we'll have to prepare for reports of breakage.

So, it's an expensive project, both in terms of BP team time and in terms of developer/user goodwill. If someone stepped forward to own this whole project, I think it'd be worth considering. If not, it feels hard to justify this project holding up other projects that would have more direct user impact.

Note: See TracTickets for help on using tickets.